jquery.ready function in layout.html should catch error

[samuel.degrande@…]

I use a plugin which adds a js function to be called after the document is loaded by using jquery(document).ready(function($) { … })

My function was not called.

After some investigation, I found that there was an exception thrown in the js function registered in layout.html.

The queue of functions registered to be launched after document is loaded is stopped if one of them does not return.

In my opinion, a try/catch should be added. See proposed patch.

[Christian Boos]

Wouldn't some kind of console.log be useful in the catch clause?

[anonymous]

Ah yes, sure. The purpose of the patch was just to show what part of layout.html I was talking about. Sorry, I should have been more precise in my comment.

[Ryan J Ollos]

Replying to samuel.degrande@univ-lille1.fr:

After some investigation, I found that there was an exception thrown in the js function registered in layout.html.

What was the exception being thrown? I'm not seeing how the code you wrapped in try/catch would raise an exception.

[anonymous]

In my plugin, I use some jquery UI plugins + the jquery validation plugin.

I can not use chrome.add_jquery_ui() due to an incompatibility between the validator plugin and jquery-ui-i18n on this line:

  yearSuffix: $.format(formatMonth, {month: '', year: ''})

(the validator plugin also defines a format() function)

So, in my plugin, I directly call add_script('common/js/jquery-ui.js').

It was working with Trac 1.1, but it does no more work with Trac 1.2 due to the js script that you added in layout.html: an exception is thrown because datetimepicker() is unknown.

The actual fix is obviously to now also add_script('/common/js/jquery-ui-addon.js') in my plugin.

But, as a guard against such issue, I was thinking that adding a try/catch could be a fine protection.

[Jun Omae]

In my plugin, I use some jquery UI plugins + the jquery validation plugin.

I can not use chrome.add_jquery_ui() due to an incompatibility between the validator plugin and jquery-ui-i18n on this line:

  yearSuffix: $.format(formatMonth, {month: '', year: ''})

(the validator plugin also defines a format() function)

Weird. The jquery validation plugin defines $.validator.format, not $.format. See ​https://github.com/jzaefferer/jquery-validation/blob/1.14.0/src/core.js#L198.

So, in my plugin, I directly call add_script('common/js/jquery-ui.js').

Instead, chrome.add_jquery_ui() should be used in Trac.

But, as a guard against such issue, I was thinking that adding a try/catch could be a fine protection.

No. I don't think we should suppress errors. Suppressing the error would lead breakages in other places.

[anonymous]

Replying to Jun Omae:

In my plugin, I use some jquery UI plugins + the jquery validation plugin.

I can not use chrome.add_jquery_ui() due to an incompatibility between the validator plugin and jquery-ui-i18n on this line:

  yearSuffix: $.format(formatMonth, {month: '', year: ''})

(the validator plugin also defines a format() function)

Weird. The jquery validation plugin defines $.validator.format, not $.format. See ​https://github.com/jzaefferer/jquery-validation/blob/1.14.0/src/core.js#L198.

I use the 1.13 version of jquery-validation, and the culprit is: ​https://github.com/jzaefferer/jquery-validation/blob/1.13.0/src/core.js#L1289

So, in my plugin, I directly call add_script('common/js/jquery-ui.js').

Instead, chrome.add_jquery_ui() should be used in Trac.

That's what I tried at first, and that's what failed. But updating to jquery-validation 1.14 will make me able to use it. Fine !

But, as a guard against such issue, I was thinking that adding a try/catch could be a fine protection.

No. I don't think we should suppress errors. Suppressing the error would lead breakages in other places.

Ok. But perhaps adding a console.log() in the catch as Christian proposed ?

Well, up to you, obviously. My issue can be fixed, so that's ok with me, and the ticket can be deleted !

Sorry I should have checked and found that the new jquery-validation is now compatible with jquery-ui-i18n.

[Ryan J Ollos]

Replying to anonymous:

But, as a guard against such issue, I was thinking that adding a try/catch could be a fine protection.

No. I don't think we should suppress errors. Suppressing the error would lead breakages in other places.

Ok. But perhaps adding a console.log() in the catch as Christian proposed ?

Well, up to you, obviously. My issue can be fixed, so that's ok with me, and the ticket can be deleted !

We'd need more information: exact steps to reproduce, the error that occurs and the error that is logged in the console after the try/catch is added. The reasonable motivation for adding the try/catch would be it allowed you to locate the problem code more quickly and we'd need that to be demonstrated. Just suppressing and logging the error so that your downstream code can execute isn't a good enough reason to add the try/catch as far as I can see.