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)
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)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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 )
Updated•15 years ago
|
Product: Firefox → Core
QA Contact: general → general
Comment 1•15 years ago
|
||
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
Reporter | ||
Comment 2•15 years ago
|
||
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
Updated•14 years ago
|
QA Contact: general → style-system
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.
Comment 5•11 years ago
|
||
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;
}
};
}
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 9•10 years ago
|
||
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.
Comment 10•10 years ago
|
||
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.
Comment 11•10 years ago
|
||
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.
Comment 12•10 years ago
|
||
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.
Comment 13•10 years ago
|
||
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.
Comment 14•10 years ago
|
||
(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.
Comment 15•10 years ago
|
||
> 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?
Comment 16•10 years ago
|
||
(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();
}
}
Comment 17•10 years ago
|
||
> if (window.getComputedStyle(domNode)) {
Is this code running in the display:none iframe?
Comment 18•10 years ago
|
||
(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.
Comment 19•10 years ago
|
||
"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.
Comment 20•10 years ago
|
||
http://webcompat.com/issues/300 documents how m.futureshop.ca is unusable due to this bug.
Severity: minor → normal
Updated•10 years ago
|
Comment 21•10 years ago
|
||
A test for reading the display property from an element inside a display:none IFRAME (should say 'block') with jQuery: http://jsfiddle.net/zn7akfxg/
Comment 24•9 years ago
|
||
If the iframe isn't displayed, couldn't getComputedStyle(node) just return a copy of node.style?
Comment 25•9 years ago
|
||
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.
Comment 26•9 years ago
|
||
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>
Comment 27•9 years ago
|
||
Could an empty object be returned instead of null? This would fix errors as most people typically do stuff like getComputedStyles(obj).color.
Updated•8 years ago
|
Comment 28•8 years ago
|
||
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)
Comment 29•8 years ago
|
||
Comment 30•8 years ago
|
||
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)
Comment 31•8 years ago
|
||
> 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.)
Comment 32•8 years ago
|
||
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)
Comment 33•8 years ago
|
||
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.
Updated•8 years ago
|
Comment 34•8 years ago
|
||
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)
Comment 35•8 years ago
|
||
> 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
Comment 36•8 years ago
|
||
This is unfortunately breaking opentable.com, which is something we'd like to fix sooner rather than later. See bug 1302780
Comment 37•8 years ago
|
||
> 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. :(
Comment 38•8 years ago
|
||
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.
Updated•8 years ago
|
Flags: webcompat?
Comment 39•8 years ago
|
||
(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.
Comment 40•8 years ago
|
||
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?
Comment 41•8 years ago
|
||
> 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.
Updated•7 years ago
|
See Also: → https://webcompat.com/issues/3791
Updated•7 years ago
|
Flags: webcompat? → webcompat+
Updated•7 years ago
|
Whiteboard: [webcompat]
Updated•7 years ago
|
Whiteboard: [webcompat] → [webcompat:p1]
Updated•7 years ago
|
See Also: → https://webcompat.com/issues/8044
Updated•7 years ago
|
See Also: → https://webcompat.com/issues/6929
Updated•7 years ago
|
User Story: (updated)
Updated•7 years ago
|
User Story: (updated)
Assignee | ||
Comment 42•7 years ago
|
||
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.
Comment 43•7 years ago
|
||
(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.
Comment 45•7 years ago
|
||
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]
Comment 46•6 years ago
|
||
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
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 48•6 years ago
|
||
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)
Comment 49•6 years ago
|
||
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)
Assignee | ||
Comment 50•6 years ago
|
||
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.
Assignee | ||
Comment 51•6 years ago
|
||
I filed bug 1471231 for the other bits mentioned in comment 48.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Comment 52•6 years ago
|
||
Marked as verified from QA perspective. Let me know if this is not correct. Thanks.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 53•6 years ago
|
||
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.
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 54•6 years ago
|
||
(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.
Comment 55•6 years ago
|
||
(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.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•