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

Status: VERIFIED DUPLICATE of bug 1467722

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

Description

[Jordan Osete]

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 )

Comment 1

[Boris Zbarsky [:bzbarsky]]

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.

Comment 2

[Jordan Osete]

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.

Comment 3

[FredCK]

Attachment: Test Case

Attached a simple test case for it. It passes on Chrome and Opera while fails with Firefox.

Comment 4

[jonl]

@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

[Boris Zbarsky [:bzbarsky]]

jonl, I have no idea what you're talking about.  Fixed width and height have no effect on this bug.

Comment 6

[jonl]

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.

Comment 7

[Ten]

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.

Comment 8

[Greg]

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;
      }
   };
}

Comment 9

[Travis Everett]

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

[Boris Zbarsky [:bzbarsky]]

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

[Travis Everett]

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

[sim]

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

[Boris Zbarsky [:bzbarsky]]

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

[sim]

(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

[Boris Zbarsky [:bzbarsky]]

> 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

[sim]

(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

[Boris Zbarsky [:bzbarsky]]

>  if (window.getComputedStyle(domNode)) {

Is this code running in the display:none iframe?

Comment 18

[sim]

(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

[Boris Zbarsky [:bzbarsky]]

"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

[Hallvord R. M. Steen [:hallvors]]

http://webcompat.com/issues/300 documents how m.futureshop.ca is unusable due to this bug.

Comment 21

[Hallvord R. M. Steen [:hallvors]]

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

[Thayne]

If the iframe isn't displayed, couldn't getComputedStyle(node) just return a copy of node.style?

Comment 25

[Boris Zbarsky [:bzbarsky]]

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

[GregAmbrose]

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

[Benjamin Tan]

Could an empty object be returned instead of null? This would fix errors as most people typically do stuff like getComputedStyles(obj).color.

Comment 28

[Boris Zbarsky [:bzbarsky]]

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?

Comment 29

[Boris Zbarsky [:bzbarsky]]

Attachment: Simple proof of concept of option 3

Comment 30

[Bobby Holley (:bholley)]

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

[Boris Zbarsky [:bzbarsky]]

> 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

[Bobby Holley (:bholley)]

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.

Comment 33

[Boris Zbarsky [:bzbarsky]]

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.

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 35

[Boris Zbarsky [:bzbarsky]]

> 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

[Johnny Stenback (:jst)]

This is unfortunately breaking opentable.com, which is something we'd like to fix sooner rather than later. See bug 1302780

Comment 37

[Boris Zbarsky [:bzbarsky]]

> 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

[Thomas Wisniewski [:twisniewski]]

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.

Comment 39

[Xidorn Quan [:xidorn] UTC+11]

(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

[Xidorn Quan [:xidorn] UTC+11]

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

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 42

[Emilio Cobos Álvarez (:emilio)]

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

[Xidorn Quan [:xidorn] UTC+11]

(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

[Panos Astithas (he/him) [:past] (please ni?)]

Downgrading to webcompat P2 as we are waiting on Blink's results per comment 42.

Comment 46

[Thomas Wisniewski]

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

Comment 47

[Boris Zbarsky [:bzbarsky]]

So is this fixed by bug 1467722 now?

Comment 48

[Emilio Cobos Álvarez (: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?

Comment 49

[Boris Zbarsky [: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.

Comment 50

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 51

[Emilio Cobos Álvarez (:emilio)]

I filed bug 1471231 for the other bits mentioned in comment 48.

Comment 52

[Levente Sacal]

Marked as verified from QA perspective. Let me know if this is not correct. Thanks.

Comment 53

[Chris Mills [:cmills]]

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.

Comment 54

[Emilio Cobos Álvarez (:emilio)]

(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

[Chris Mills [:cmills]]

(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.