Closed Bug 548397 Opened 15 years ago Closed 6 years ago

window.getComputedStyle() returns null inside an iframe with display: none

Categories

(Core :: DOM: CSS Object Model, defect, P3)

x86
Linux
defect

Tracking

()

VERIFIED DUPLICATE of bug 1467722

People

(Reporter: jordan.osete, Assigned: emilio)

References

(Depends on 1 open bug, Blocks 2 open bugs, )

Details

(Keywords: dev-doc-complete, Whiteboard: [webcompat:p2])

User Story

Business driver: Long-tail interop (with some other top sites) + common dev papercut

Note: Likely pretty safe according to bz.

Attachments

(2 files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 getComputedStyle() returns null if the page is loaded in an iframe with display: none; Reproducible: Always Steps to Reproduce: 1. go to http://ministeyr.free.fr/testcases/gcs2.htm Actual Results: The page alerts "null" Expected Results: The page should alert something like [object CSSStyleDeclaration] It happens correctly if the page is not loaded inside an iframe (see http://ministeyr.free.fr/testcases/gcs.htm )
Product: Firefox → Core
QA Contact: general → general
If you're display:none then the information needed to compute style (which medium is in use, the size of the CSS viewport, the zoom level, etc) is not present. So yes, this is the expected behavior given the current style computation implementation in Gecko. The CSSOM spec might require a different behavior at some point, but in the meantime this is something to bring up with the relevant working group.
Component: General → DOM: CSS Object Model
QA Contact: general → general
A simple workaround is to style the iframe with visibility: hidden; position: absolute; Although "display: none" probably impacts performance less, but this should not be very significant.
Severity: normal → minor
QA Contact: general → style-system
Attached file Test Case (deleted) —
Attached a simple test case for it. It passes on Chrome and Opera while fails with Firefox.
Attachment #641808 - Attachment mime type: text/plain → text/html
@Boris, Your statement does not make sense. Another workaround to this issue is to provide the iframe with a fixed width and height. If you do that, then even if the style display:none is set, getComputedStyle will not return null.
jonl, I have no idea what you're talking about. Fixed width and height have no effect on this bug.
I retract my comment. I have run into this with my own application and thought I had seen different behavior with a fixed size iframe, but appear to be mistaken.
I ran into this bug with a piece of software I'm developing and found a solution to it. In my code I was dynamically changing the src property of an iframe that was located inside a jquery ui dialog so this might be situational. I set a micro timeout 'window.setTimeout(fuction() { });' around the code that changed the 'src' property of the iframe and it works every time now without throwing this error.
I have make a patch for my application which use jquery.mobile in an iframe. I did not want change the behaviour of the jquery.mobile to assure the compatibilty with updates of library. I have overload the getComputedStyle function by checking the result and replace it by an empty object in case it is null. if (/firefox/i.test(navigator.userAgent)){ window.oldGetComputedStyle = window .getComputedStyle; window.getComputedStyle = function (element, pseudoElt) { var t = window.oldGetComputedStyle(element, pseudoElt); if (t === null) { return {}; } else{ return t; } }; }
Status: UNCONFIRMED → NEW
Ever confirmed: true
We ran into this bug while updating versions of Adobe Captivate. Captivate warns against use of browsers other than chrome/IE/Safari, but it seems whatever issues they had with FF are largely in the past. Still, when we include a Captivate (versions 7/8, at least) in an unopened iframe until the user clicks to display the iframe in a modal, this issue crops up. I'm not involved in the spec here enough to _know_ if it's naive of Adobe to be making assumptions (i.e., getComputedStyle(el) will return an object with top/right/bottom/left/marginTop/marginRight/marginBottom/marginLeft/width/height values) but it seems like the spec (http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSview-getComputedStyle) gives no indication that a null return value should be anticipated.
The relevant spec is http://dev.w3.org/csswg/cssom/ for what it's worth. But yes, returning null is probably wrong, which is why this bug is open. Fixing it requires pretty significant changes to how style computation works in Gecko, unfortunately.
Thanks for the quick update, Boris. We used a modified version of Greg's patch above to return a dummy object with values for the necessary keys to cover our case in the near term; largely posting to document the interaction between this issue and Captivate, in case it adds some helpful weight to the case for an eventual fix.
We are seeing window.getComputedStyle() return null for DOM elements that are not inside iframes. This is happening thousands of different sites for millions of users where our scripts are running. FF only behavior; not happening on any other browser. Unfortunately, we haven't been able to isolate a self-contained test case yet. Still, for the purposes of considering a fix we wanted to make sure that this use case demonstrating the problem is known.
sim, I'm not aware of a way for getComputedStyle to return null unless the method is being called on a window with no presentation. That means it's either a display:none iframe (or some iframe that has no box; e.g. a direct child of a non-foreignObject SVG element) or a tab that was somehow hidden by some extension.
(In reply to Boris Zbarsky [:bz] from comment #13) Well, it's happening with clockwork consistency. While I cannot give you an isolated reproducible case, I'd be happy to screen share and demonstrate. Here is an interesting twist, which hopefully sheds some light. The DOM element being manipulated and the window reference come from the main frame of web page (not iframed) but the JavaScript calling the method on that window reference was loaded in a same-domain hidden iframe.
> but the JavaScript calling the method on that window reference was loaded in a > same-domain hidden iframe. What does that javascript call look like, exactly?
(In reply to Boris Zbarsky [:bz] from comment #15) Here is the current code (with a workaround for this FF issue). We got bitten by this issue when attempting to show a hidden DOM element. The DOM element in question is a DIV in the main frame with display: none. When window.getComputedStyle() returns null, jQuery's fadeIn() fails to make the element appear but show() does alright. _makeVisible: function() { var domNode = this.$ele.get(0); if (window.getComputedStyle(domNode)) { this.$ele.fadeIn(200); } else { this.$ele.show(); } }
> if (window.getComputedStyle(domNode)) { Is this code running in the display:none iframe?
(In reply to Boris Zbarsky [:bz] from comment #17) Yes, the code is running in the display: none same-domain iframe. However, window references the top-level frame on the page.
"window" in that function references the window the code is running in, which you say is the display:none iframe, unless the function is closing over some scope with "var window" in it.
http://webcompat.com/issues/300 documents how m.futureshop.ca is unusable due to this bug.
Severity: minor → normal
A test for reading the display property from an element inside a display:none IFRAME (should say 'block') with jQuery: http://jsfiddle.net/zn7akfxg/
If the iframe isn't displayed, couldn't getComputedStyle(node) just return a copy of node.style?
That has quite different behavior from the computed style, unfortunately. In particular, it doesn't have values for properties that are not actually set in the inline style.
I bypassed the problem very easily with JQuery as follows. <div> <iframe id='linkIframe' width='1200px' height='300px'></iframe> </div> <script> $(document).ready(function() { $("#linkIframe").attr("src", "/iadmin2/plugins/jobcard/externalMedia.php"); }); </script>
Could an empty object be returned instead of null? This would fix errors as most people typically do stuff like getComputedStyles(obj).color.
So I ran into yet another site today that breaks as a result of this. I think we have the following options at this point: 1) Wait for stylo to land, try to fix this on top of stylo. At that point style computation won't be quite as tied to the presshell, and we might be able to just make it work. I'm not sure what sort of timeframe this would entail. 2) Divorce style computation from the presshell. That is, move the style struct arenas, style set, etc, etc over to the document. This would take quite a bit of work in its own right... 3) Hack something together where we return non-null (this part is easy) and either have it throw on access like happens now if you grab a computed style object and then go display:none or have it return some sort of bogus data (e.g. as suggested in comment 27). The former option does _not_ fix the site I mention above, but the latter one does. Just returning "" for everything is actually fairly simple to do. David, thoughts? Bobby, do we have any sort of firm-ish time estimate for stylo?
Flags: needinfo?(dbaron)
Flags: needinfo?(bobbyholley)
The comments in the bug don't make it entirely clear what the issue is (and which parts are expected/unexpected per-spec) - Boris, can you elaborate on that? (I'll be slow to respond, in TPE for stylo meetup)
> Boris, can you elaborate on that? The issue is that Gecko's style system can't compute style for things in a display:none iframe, because style computation is tied to the presshell, and there isn't one. Though maybe the answer is that there should in fact be a presshell but frame construction should be suppressed. (Or allowed, with an assumption of 0x0 size for the presshell; arguably that's what the CSS spec calls for, but it's ridiculously inefficient and I don't think we want to go there.)
I think Stylo doesn't really offer a super-clean way to fix this. Most of the per-document state is set up via presshell initialization, and we can't construct style structs without a prescontext. So I think the issue is mostly orthogonal. If it's invasive and not urgent I'd still rather we punt on it for now though, since it'll touch a lot of the same code.
Flags: needinfo?(bobbyholley)
OK. So I talked to dbaron, and he's on board with the plan in comment 31. I don't think changing that will affect stylo much, if any. This came up again today in https://github.com/hakimel/reveal.js/issues/1546 though it's not clear to me whether the comment 31 approach will actually fix that site, because it might still result in things looking 0-sized.
Blocks: 1302780
Depends on: 1308675
Yeah, having a pres shell seems like fewer special cases. That said, maybe it's worth pinging the Chromium folks and seeing if they're interested in trying to change their behavior, given that not constructing things when display:none seems like a more efficient behavior.
Flags: needinfo?(dbaron)
> maybe it's worth pinging the Chromium folks and seeing if they're interested in trying > to change their behavior Did that already. See https://github.com/whatwg/html/issues/1813
This is unfortunately breaking opentable.com, which is something we'd like to fix sooner rather than later. See bug 1302780
> This is unfortunately breaking opentable.com Yes, that's the site that led me to get back to this in comment 28. I've been working on this on and off per my plan from comment 31, but what I have so far is _very_ orange on try: see <https://treeherder.mozilla.org/#/jobs?repo=try&revision=88f5506602d5bd66d688fc8cabd3ffa58d9a5916>. I've slowly been working through some of the failures, but I could drop other things for a bit and really focus on this if we think the opentable breakage is important enough. Unfortunately it's failures in all sorts of different parts of the code, not just a case of the same failure happening in lots of test suites. :(
Note that sites using jQuery 2.1 and 1.11 could be affected by this as well ever since bug 1355683 landed. I'm not sure if that's a big enough deal to revert bug 1355683 or not, but it may be worth considering.
Flags: webcompat?
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #37) > > This is unfortunately breaking opentable.com > > Yes, that's the site that led me to get back to this in comment 28. > > I've been working on this on and off per my plan from comment 31, but what I > have so far is _very_ orange on try: see > <https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=88f5506602d5bd66d688fc8cabd3ffa58d9a5916>. I've > slowly been working through some of the failures, but I could drop other > things for a bit and really focus on this if we think the opentable breakage > is important enough. Unfortunately it's failures in all sorts of different > parts of the code, not just a case of the same failure happening in lots of > test suites. :( The result does not exist now, but IIRC there was an assumption in some of nsComputedDOMStyle code that, when the style flushed, frame construction is supposed to be done as well, which is true at the moment in Gecko, but presumably changes with your patch.
Another option, I guess, is that we create pres shell with 0x0 viewport and trigger frame construction for display:none iframe like normal, but only do that when someone tries to call getComputedStyle on window of that, which means do that lazily. Would that be easier to implement?
> Note that sites using jQuery 2.1 and 1.11 could be affected by this as well ever since bug 1355683 landed. getDefaultComputedStyle had the same behavior, no? I don't think there's any real change. > which is true at the moment in Gecko, but presumably changes with your patch. That's true, but only for cases in which we had no presshell at all. There were all sorts of test failures in tests that had nothing to do with computed style. > is that we create pres shell with 0x0 viewport and trigger frame construction for display:none iframe > like normal, but only do that when someone tries to call getComputedStyle on window of that, which means > do that lazily. That would make various other DOM apis whose return value depends on the box existing or not (e.g. getBoundingClientRect) return different values depending on whether getComputedStyle happened. That doesn't seem very desirable.
Flags: webcompat? → webcompat+
Whiteboard: [webcompat]
Depends on: 1418159
Depends on: 1430608
Depends on: 1430623
Depends on: 1432490
Whiteboard: [webcompat] → [webcompat:p1]
Depends on: 1452080
User Story: (updated)
User Story: (updated)
Just FYI, the CSSWG discussed here in https://github.com/w3c/csswg-drafts/issues/2403 / https://github.com/w3c/csswg-drafts/issues/1548, and Blink wants to try not to compute style in display: none / non-rendered iframes, and return the empty string instead as they do for detached nodes... I'm not totally optimistic that it's going to work out, but if it does it surely would allow us to simplify a bunch of stuff and handle this bug the same way instead of returning null.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #42) > I'm not totally optimistic that it's going to work out, but if it does it > surely would allow us to simplify a bunch of stuff and handle this bug the > same way instead of returning null. I don't think that's going to work given our previous attempt on unshipping getDefaultComputedStyle.
Downgrading to webcompat P2 as we are waiting on Blink's results per comment 42.
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Whiteboard: [webcompat:p1] → [webcompat:p2]
Just noting that resolutions have since been posted in https://github.com/w3c/csswg-drafts/issues/1548 : >RESOLVED: For elements not part of a tree or part of a detached tree they return no computed styles >RESOLVED: Elements not part of a flat tree return no style for getComputedStyle >RESOLVED: Regardless of what window you obtain the gCS function from the origin of the style is where ever the element is >RESOLVED: getComputedStyle returns none for elements part of non-rendered iframes
Depends on: 1467722
Priority: -- → P3
So is this fixed by bug 1467722 now?
Flags: needinfo?(emilio)
Well, the "don't return null" bit, yes, I guess. But we still aren't completely compatible with other engines, as-in, they would return a meaningful style for an element in a display: none iframe but we wouldn't... Not sure if we should re-title or close this and file a bug for that... wdyt?
Flags: needinfo?(emilio) → needinfo?(bzbarsky)
I'd be tempted to go with a new bug. Worth checking the various things this bug blocks or are "See Also" to see which of them really care about the value (and should depend on the new bug) and which are fixed by just not throwing.
Flags: needinfo?(bzbarsky)
Yeah. Some of them would be clearly fixed by the already done work in bug 1467722, like the ones that detect adblock if getComputedStyle().display == 'none' || getComputedStyle().visibility == 'hidden'; Others are not, like https://github.com/webcompat/web-bugs/issues/13733 for example.
Blocks: 1471231
I filed bug 1471231 for the other bits mentioned in comment 48.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Marked as verified from QA perspective. Let me know if this is not correct. Thanks.
Status: RESOLVED → VERIFIED
We do report this bug as existing in https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle#Browser_compatibility, but it is reported as being fixed by Firefox 62, which doesn't sound right to me. I've removed the BCD note (https://github.com/mdn/browser-compat-data/pull/2735) and added an updated note into the text body instead about it still being problematic. I don't think this is really fixed and worth removing the note altogether until https://bugzilla.mozilla.org/show_bug.cgi?id=1471231 is resolved.
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #53) > We do report this bug as existing in > https://developer.mozilla.org/en-US/docs/Web/API/Window/ > getComputedStyle#Browser_compatibility, but it is reported as being fixed by > Firefox 62, which doesn't sound right to me. I've removed the BCD note > (https://github.com/mdn/browser-compat-data/pull/2735) and added an updated > note into the text body instead about it still being problematic. > > I don't think this is really fixed and worth removing the note altogether > until https://bugzilla.mozilla.org/show_bug.cgi?id=1471231 is resolved. Note that bug 1471231 is a different bug, and not only that but it's what other browsers agreed to do on https://github.com/w3c/csswg-drafts/issues/1548.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #54) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #53) > > We do report this bug as existing in > > https://developer.mozilla.org/en-US/docs/Web/API/Window/ > > getComputedStyle#Browser_compatibility, but it is reported as being fixed by > > Firefox 62, which doesn't sound right to me. I've removed the BCD note > > (https://github.com/mdn/browser-compat-data/pull/2735) and added an updated > > note into the text body instead about it still being problematic. > > > > I don't think this is really fixed and worth removing the note altogether > > until https://bugzilla.mozilla.org/show_bug.cgi?id=1471231 is resolved. > > Note that bug 1471231 is a different bug, and not only that but it's what > other browsers agreed to do on > https://github.com/w3c/csswg-drafts/issues/1548. Noted; I've improved it since then.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: