Opened 12 years ago

Closed 10 years ago

#5197 closed defect (fixed)

(Firefox only) hidden iframes can throw exceptions when testing dojo._isBodyLtr()

Reported by: guest Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Dijit Version: 1.0
Keywords: bidi Cc: hwcdl@…, stenduncan@…, alex, dante, James Burke, Douglas Hays, Adam Peller
Blocked By: Blocking:

Description (last modified by Adam Peller)

the dojo._isBodyLtr() is added to my custom code base when I build (presumably by dijit), i'm not sure where this added:

if(!dojo._hasResource["dijit._base.bidi"]){ //_hasResource checks added by build. Do not use _hasResource directly in your code.
dojo._hasResource["dijit._base.bidi"] = true;
dojo.provide("dijit._base.bidi");

// summary: applies a class to the top of the document for right-to-left stylesheet rules

dojo.addOnLoad(function(){
	if(!dojo._isBodyLtr()){
		dojo.addClass(dojo.body(), "dijitRtl");
	}
});

In anycase when this is called the subsequent dojo._bodyLtr = dojo.getComputedStyle(dojo.body()).direction == "ltr" returns null on an iframe that is wrapped in a hidden div (see attached test files).

Attachments (3)

hiddenIframe.html (872 bytes) - added by guest 12 years ago.
test.html (300 bytes) - added by guest 12 years ago.
5197.patch (621 bytes) - added by Douglas Hays 10 years ago.
possible fix using dir attribute instead of calling gcs

Download all attachments as: .zip

Change History (23)

Changed 12 years ago by guest

Attachment: hiddenIframe.html added

Changed 12 years ago by guest

Attachment: test.html added

comment:1 Changed 12 years ago by guest

For some reason my email doesn't remain in the form it's (ialpert at coemergence dot com). To use the test load hiddenIframe.html in firefox with test.html in the same directory (test.html is the source of the iframes used in hiddenIframe.html). You'll probalby have to change the location of the dojo libraries

comment:2 Changed 12 years ago by Adam Peller

Cc: hwcdl@… added
Keywords: bidi added
Owner: set to Adam Peller

comment:3 Changed 12 years ago by guest

Might be related to #5142: dojo.style, undefined or null error

comment:4 Changed 12 years ago by bill

Milestone: 2.0

comment:5 Changed 12 years ago by bill

Yeah, getting style on hidden nodes doesn't work well so this is probably a "wonfix" (or "cantfix"). You should try on FF3 to see if it works better there.

comment:6 Changed 12 years ago by guest

Maby a reasonable fix for this is a null guard in the calling code {{{ dojo._isBodyLtr = function(){

FIXME: could check html and body tags directly instead of computed style? need to ignore case, accept empty values return !("_bodyLtr" in dojo) ?

dojo._bodyLtr = dojo.getComputedStyle(dojo.body()).direction == "ltr" : dojo._bodyLtr; Boolean

}

}}}

comment:7 in reply to:  5 Changed 12 years ago by guest

Replying to bill:

Yeah, getting style on hidden nodes doesn't work well so this is probably a "wonfix" (or "cantfix"). You should try on FF3 to see if it works better there.

I am seeing a similar problem as the original poster - I have a TabContainer? where each "tab" is a ContentPane? with no content originally. When my app runs, each ContentPane? gets the content set with an iframe (which has a src tag).

This all ran perfectly fine in 0.4.3, but I am now upgrading to 1.0.1 and when my first frame is being loaded (it is not the selected frame), I am getting the error:

dojo.getComputedStyle(dojo.body()) has no properties

dojo._bodyLtr = dojo.getComputedStyle(dojo.body()).direction == "ltr" :

(for me it's in dojo.js.uncompressed.js line 4306).

and my dojo.addOnLoad function is never getting called so my code does not initialize properly.

This is a pretty severe problem and I don't see any workaround except hacking away at the dojo source code. Is there any chance that someone else has a workaround or that we'll get a solution soon?

If possible, I'd like to be added to the cc on this bug (stenduncan @ gmail dot com)

comment:8 Changed 12 years ago by Adam Peller

Cc: stenduncan@… added

comment:9 Changed 12 years ago by bill

Stenduncan - I'm wondering why you need to use iframes in the first place, since ContentPane? has similar functionality to an iframe, with href loading. But if you need to use an iframe for each tab, then why don't you just wait to load each tab until the user selects it? (ie, lazy loading)

comment:10 Changed 12 years ago by Adam Peller

Cc: alex added

so a related issue is whether this code in dojo.loaded (loader.js) should have a try/catch on each iteration:

		for(var x=0; x<mll.length; x++){
			mll[x]();
		}

comment:11 in reply to:  9 Changed 12 years ago by guest

Replying to bill:

Stenduncan - I'm wondering why you need to use iframes in the first place, since ContentPane? has similar functionality to an iframe, with href loading. But if you need to use an iframe for each tab, then why don't you just wait to load each tab until the user selects it? (ie, lazy loading)

Bill, you're absolutely right, I probably don't need iframes and I'm going to be changing that in the future. But I am doing a straight port from 0.4.3 to 1.0.1 at the moment and I smashed into this. I don't have time to rewrite my entire app just now.

I've edited my copy of the Dojo code directly (I hate doing that) to make sure I don't get that error any more, but I'd love to hear that this has been fixed in the real code.

-- stenduncan

comment:12 Changed 12 years ago by Adam Peller

(In [11852]) put onload trigger in try/catch so a failure doesn't kill the bootstrap. Refs #5197

comment:13 Changed 12 years ago by alex

Milestone: 2.01.3

Milestone 2.0 deleted

comment:14 Changed 12 years ago by Adam Peller

(In [12776]) add more descriptive error message for onload failure. Refs #5197

comment:15 Changed 11 years ago by Adam Peller

Description: modified (diff)
Milestone: 1.3future

comment:16 Changed 10 years ago by Adam Peller

Cc: dante James Burke added
Milestone: future1.4

see Dean's article http://deanedwards.me.uk/weblog/2009/03/callbacks-vs-events/ Perhaps this should be addressed in a separate ticket, but I'd like to see if we can revisit this for 1.4.

Changed 10 years ago by Douglas Hays

Attachment: 5197.patch added

possible fix using dir attribute instead of calling gcs

comment:17 Changed 10 years ago by Douglas Hays

Cc: Douglas Hays added

Another customer reported this same problem with FF3. They cannot use a hidden ContentPane? in the main HTML file since dojo is only loaded in the IFRAME. They get an exception on page load. I attached a patch to _isBodyLtr that seems to fix the problem.

comment:18 Changed 10 years ago by bill

Cc: Adam Peller added

The patch looks good to me.

I tried to find a case where getComputedStyle() returns "rtl" even though dir=rtl isn't explicitly marked on <body> (I thought, maybe the browser can guess the page direction based on the page's language setting, charset, etc.), but I can't find an example. I looked at http://www.haaretz.co.il/ and Firefox says the computed style of the page is ltr... so this change wouldn't make things worse. Seems safe to me.

comment:19 Changed 10 years ago by Douglas Hays

Owner: changed from Adam Peller to Douglas Hays
Status: newassigned

comment:20 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [19113]) Fixes #5197 !strict. Change _isBodyLtr to inspect DOM nodes instead of computed style. Add html_isBodyLtr.html automated tests.

Note: See TracTickets for help on using tickets.