Opened 7 years ago

Closed 7 years ago

#15794 closed defect (fixed)

i18n improperly switching to async mode while servicing sync i18n API

Reported by: bill Owned by: Rawld Gill
Priority: blocker Milestone: 1.8
Component: Internationalization Version: 1.7.3
Keywords: Cc: Rawld Gill, Adam Peller
Blocked By: Blocking:

Description

Can be reproduced from http://archive.dojotoolkit.org/nightly/dojotoolkit/demos/dynamicChart/demo.html.

I also reproduced locally after doing a build as:

$ cd util/buildscripts
$ ./build.sh action=release profile=demos-all optimize=comments.keeplines layerOptimize=comments.keeplines releaseDir=/workspace/release-demos

and then loading /release-demos/demosite/dynamicChart/demo.html in FF or chrome (and maybe other browsers too). But it was working for Christophe locally.

Problem happens loading NLS resource dijit/form/nls/validate. It calls evalBundle(), which calls xhr.get() sync mode:

xhr.get({url:url, sync:true, load:load, error:function () {
results.push(cache[url] = {});

but the results.push(... callback code is not executed right away.

In particular, the callback code in doLoad() below, executes before the callback code above, leading to an exception since root is undefined:

require([bundlePathAndName], function (root) { ... })

Change History (11)

comment:1 Changed 7 years ago by bill

Milestone: tbd1.8
Priority: undecidedblocker

comment:3 Changed 7 years ago by cjolif

I did not reproduce because I had not seen I had local modifications to dojo/request which prevented my code to merge with latest. Using latest dojo/request I do reproduce. This hints that the problem may have been relatively recently introduced by dojo/request changes.

Last edited 7 years ago by cjolif (previous) (diff)

comment:4 Changed 7 years ago by cjolif

Well on the other hand putting back my local version of dojo/request does not seem to make it back working, so maybe this is not related... Maybe I just forgot to clean something. In any case what is 100% sure is that it was working a few days/weeks ago.

comment:5 Changed 7 years ago by cjolif

Cc: Rawld Gill added

One of the difference between the 7/1 build and the 8/1 one is that on the 1/7 the demo src.js was not built as a layer (just contain its own source not all the dependencies). This is because at that point in time the demo was (wrongly) commented out from the demos-all profile. That might explain why we have not noticed that problem sooner... That said it does not explain why it fails once it is built as a layer... Maybe rawld would have hints there?

Last edited 7 years ago by cjolif (previous) (diff)

comment:6 Changed 7 years ago by Rawld Gill

Component: IOInternationalization
Owner: changed from Bryan Forbes to Rawld Gill
Status: newassigned

i18n uses sync mode to implement the legacy synchronous i18n API (e.g, getLocalization), even if the global mode is async. If the bundle is actually AMD, it incorrectly switches back to async mode if the global mode is async. This can cause a bundle to not load correctly is some circumstances.

comment:7 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [29426]:

stay in sync mode across i18n legacy sync API; fixes #15794; !strict

comment:8 Changed 7 years ago by bill

Cc: Adam Peller added
Resolution: fixed
Status: closedreopened

Except, starting in [29426] the I18N tests fail, try running util/doh/runner.html?test=dojo/tests/i18n (on either IE8 or FF). Both the i18n.extra.sync and i18n.extra.async tests fail.

comment:9 Changed 7 years ago by cjolif

I suspect this is "just" the test case using the private evalBundle method that has not been updated to take into account the new parameter.

comment:10 Changed 7 years ago by Rawld Gill

Summary: xhr.get() sync mode broken (?)i18n improperly switching to async mode while servicing sync i18n API

comment:11 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

In [29431]:

updated test to work with [29426] and removed some test noise; fixes #15794; !strict

Note: See TracTickets for help on using tickets.