Javascript errors / Upgrade errors

Discussion corner for Developers of Serendipity.
Post Reply
User avatar
garvinhicking
Core Developer
Posts: 30014
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Javascript errors / Upgrade errors

Post by garvinhicking » Mon Mar 02, 2015 12:03 pm

Hey,

the blogs that I have attended for upgrades from 1.x to 2.x which did not run properly (4 blogs) all showed the same issue.

The new parsing of javascript through Smarty can easily be screwed up by PHP errors, i.e. by wrong config.inc.php files of a template (emitting PHP notices) or by malformed paths, or by outdated plugins. In those cases, the whole javascript file will be invalid.

Users usually saw no proper indication unless they opened their JS console or the JS file URL manually; most usually people wondered why the expanding/collapsing of items wouldn't work.

To remedy this I tried to find a way to at least notice users that something wasn't working properly. For that I added a check to the overview.tpl file that uses a JS check to see whether serendipity.spawn exists or not, and then show an error message.

Commit is: https://github.com/s9y/Serendipity/comm ... e327c35fc2

What do you think?
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

User avatar
onli
Regular
Posts: 2230
Joined: Tue Sep 09, 2008 10:04 pm
Contact:

Re: Javascript errors / Upgrade errors

Post by onli » Mon Mar 02, 2015 12:51 pm

Good idea!

I only have some comments on the specific implementation.

1. We could move the javascript to the head of the backend and show the error on every page in the message box at the top. An error like this would imho be too severe to show it only in the dashboard, where it is quite possible to miss.

2. I dislike the $debug-variable introduced in the same commit. Part of that is purely aesthetical: Much code given how small serendipity_smarty_show is. And how important clear readability of the function. The other is that I can't see the purpose. In the specific case for example: serendipity_editor.js.tpl is only created at one specific place. Why echo that place via the core and not at the place itself?

2.1 Further: Why not just show the debug-information based on the global debug level, instead of toggling it via the function call? Edit: More importantly: If that is something you want to have in place in general - probably, given you added it to the core - why not use the logger?

2.2 The $debug-variable itself seems misnamed to me - debugorigin might be clearer

*Edit:* In general, we might think about additionally falling back to a static version of the serendipity_editor.js. Then we need to update that file again though.

User avatar
garvinhicking
Core Developer
Posts: 30014
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Javascript errors / Upgrade errors

Post by garvinhicking » Mon Mar 02, 2015 5:13 pm

Hi!
1. We could move the javascript to the head of the backend and show the error on every page in the message box at the top. An error like this would imho be too severe to show it only in the dashboard, where it is quite possible to miss.
I thought about that too, but thought that having it on each page could affect the upgrader/installer, and/or disturb the page layout...so I thought to better have it on a fixed specific place. And IMO it's visually very obvious to have it on the loginscreen...?!
2. I dislike the $debug-variable introduced in the same commit. Part of that is purely aesthetical: Much code given how small serendipity_smarty_show is. And how important clear readability of the function. The other is that I can't see the purpose. In the specific case for example: serendipity_editor.js.tpl is only created at one specific place. Why echo that place via the core and not at the place itself?
Time and again I was unable to properly see where the files where generated through which logic, so I wanted to dynamically append compilation info to files, and possible in the future for other files where I could not find the proper source.

Also if in the future people created their own serendipity_editor.js.tpl a static information inside the file would not lead to the REAL file, and even more confusion.
2.1 Further: Why not just show the debug-information based on the global debug level, instead of toggling it via the function call? Edit: More importantly: If that is something you want to have in place in general - probably, given you added it to the core - why not use the logger?
Because I wanted it in that place always, by default in each installation. People who need to fix that file should see the actual place where it's being read from; if they first needed to enable some debug mode, that would make it too hard.
2.2 The $debug-variable itself seems misnamed to me - debugorigin might be clearer
Yeah well... debugorigin looks ugly and I wanted a short varname ;) But I'm open to renaming it.
*Edit:* In general, we might think about additionally falling back to a static version of the serendipity_editor.js. Then we need to update that file again though.
I thought about that too, but wasn't sure how to do that without having outdated files or redundancies that create even more trouble in maintenance than they solve...
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/

Post Reply