Last Comment Bug 412679 - iframe not using intrinsic height
: iframe not using intrinsic height
Status: RESOLVED FIXED
[really duplicate of bug 363675]
:
Product: Core
Classification: Components
Component: Layout: R & A Pos (show other bugs)
: Trunk
: x86 All
: -- normal with 1 vote (vote)
: ---
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
: 363675 409079 428344 (view as bug list)
Depends on: 414321
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-16 13:56 PST by Dennis Bell
Modified: 2008-04-11 17:46 PDT (History)
14 users (show)
mtschrep: blocking1.9+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase from comment #0 (851 bytes, application/xhtml+xml)
2008-01-16 16:31 PST, Brian Polidoro
no flags Details
This fixes it (5.19 KB, patch)
2008-01-16 21:47 PST, Boris Zbarsky [:bz]
dbaron: review+
dbaron: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review
Fix XUL stuff (15.30 KB, patch)
2008-01-25 20:48 PST, Boris Zbarsky [:bz]
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review

Description Dennis Bell 2008-01-16 13:56:22 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2

If FF2, an absolutely positioned iframe with CSS styles of width set to auto and left and right set to 0 will resize to fill the containing absolutely-positioned element (in this case, a div).  This does not work in FF3.  However, both FF2 and FF3 expand the height when top and bottom CSS styles are specified.

Reproducible: Always

Steps to Reproduce:
Use the following code snippet as the source, and open in FF2 and FF3 to compare:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
  <head>
    <style type="text/css"> /*<![CDATA[*/
      body {background-color: #888;}
      iframe {
        position:absolute;
        top:0px;
        left:0px;
        right:0px;
        bottom:0px;
        border: 2px inset wheat;
      }
      div#container {
        position:absolute;
        top: 5em;
        left: 5em;
        right: 5em;
        bottom:5em;
        border: 2px solid black;
        background-color: wheat;
      }
    /*]]>*/
    </style>
    <title></title>
  </head>
  <body>
    <div id="container">
      <iframe id="content" src="http://www.getfirefox.com" name="content"></iframe>
    </div>
  </body>
</html>

Actual Results:  
The iframe responds to CSS style settings for top and bottom by filling the containing div from top to bottom in both browsers, but only filled the container from left to right in the FF2 browser, not the FF3 browser.  Resizing the browser's windows result in the height and width of the iframe to match that of the containing div in FF2, but only the height of the iframe and not the width resize to match the containing div in FF3

Expected Results:  
The FF3 browser's rendering of the page should look like that of the FF2's rendering,
Comment 1 Brian Polidoro 2008-01-16 16:31:31 PST
Created attachment 297456 [details]
testcase from comment #0
Comment 2 Brian Polidoro 2008-01-16 16:33:26 PST
*** Bug 409079 has been marked as a duplicate of this bug. ***
Comment 3 Matthew Cline 2008-01-16 17:32:49 PST
Confirming with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011504 Minefield/3.0b3pre
Comment 4 Brian Polidoro 2008-01-16 19:05:36 PST
The same thing happens in Opera 9.25.  And I think it's rare that both Mozilla and Opera are wrong.  

If I'm interpreting CSS 2.1 correctly then I think this bug is invalid.  Remember an iframe is a replaced element.  And the result in the testcase is an iframe that is 300 pixels wide.

from 10.3.8 Absolutely positioned, replaced elements:
"1. The used value of 'width' is determined as for inline replaced elements. "

I'm not sure but I think this is what's happening here:

from 10.3.2 Inline, replaced elements:
"Otherwise, if 'width' has a computed value of 'auto', but none of the conditions above are met, then the used value of 'width' becomes 300px. If 300px is too wide to fit the device, UAs should use the width of the largest rectangle that has a 2:1 ratio and fits the device instead."

Setting width:100%; on the iframe does get the desired behavior.  
Comment 5 philippe (part-time) 2008-01-16 19:21:37 PST
(In reply to comment #4)

I was about to comment the same as you. With the following addition:
1. WebKit behaves nearly the same as Opera 9.25 (shrink-wrapped width).
2. Opera 9.5b and WebKit: the height of the iframe is different: about 150px tall (they don't take the 'bottom' value into consideration): a 2:1 ratio.

WebKit: latest nightly build and Safari 3.04, running on 10.4.11.
Comment 6 Brian Polidoro 2008-01-16 20:07:48 PST
I was wondering about the height.  But I have no idea if there is an intrinsic ratio for an iframe. The 2:1 given only applies if device is less than 300pxs wide.  (Like perhaps a handheld device) 
Comment 7 philippe (part-time) 2008-01-16 20:24:39 PST
(In reply to comment #6)
10.6.5 applies here, which points to 10.6.2 (as 'height' of the iframe is auto), where you'll see a paragraph mentioning the 2:1 ratio, and 150px.
Comment 8 Boris Zbarsky [:bz] 2008-01-16 21:17:38 PST
The width thing is a bug in Fx2.  Comment 4 is spot on about what the spec says here.  That part is basically a duplicate of bug 410146.

The height part is a bug, just like bug 385870.

The 2:1 thing is interesting; it wasn't there when I last looked at the spec.  Yay mutating specs!  But it's not relevant: iframes have a 150px intrinsic height.
Comment 9 Dennis Bell 2008-01-16 21:23:23 PST
(In reply to comment #4)
> The same thing happens in Opera 9.25.  And I think it's rare that both Mozilla
> and Opera are wrong.  
> 
> If I'm interpreting CSS 2.1 correctly then I think this bug is invalid. 
> Remember an iframe is a replaced element.  And the result in the testcase is an
> iframe that is 300 pixels wide.
> 
> from 10.3.8 Absolutely positioned, replaced elements:
> "1. The used value of 'width' is determined as for inline replaced elements. "
> 
> I'm not sure but I think this is what's happening here:
> 
> from 10.3.2 Inline, replaced elements:
> "Otherwise, if 'width' has a computed value of 'auto', but none of the
> conditions above are met, then the used value of 'width' becomes 300px. If
> 300px is too wide to fit the device, UAs should use the width of the largest
> rectangle that has a 2:1 ratio and fits the device instead."
> 
> Setting width:100%; on the iframe does get the desired behavior.  
> 

There appears to be a conflict in section 10.3, so I think section 9.2 takes
over as authoritative.  Let me explain:

First, section 10.3.8 specifies: 
  "This situation is similar to the previous one, except that the element has
an intrinsic width."

Then, in section 10.3.2, we can only get to the clause "Otherwise, if 'width'
has a computed value of 'auto', but none of the conditions above are met, then
the used value of 'width' becomes 300px", is if the element DOES NOT have an
intrinsic value (as the case where there is an intrinsic width is handled above
this clause)

For years, naked iframes have been rendered as 150x300, but that is due to this
misinterpretation.  Iframes, basically HTML pages, do not have intrinsic
dimensions (if it did, you shouldn't be able to resize your browser).  Infact,
the W3C standard for HTML 4.01 does not specify a default height or width.

Therefore, since we cannot follow the rules in 10.3.8 to generate a width (no
intrisic dimensions), we can follow the rules as if it was a non-replacement
element and follow the rules of 9.2 for positioned elements.
Comment 10 Boris Zbarsky [:bz] 2008-01-16 21:33:03 PST
> For years, naked iframes have been rendered as 150x300

Yes.  That's the intrinsic size of an iframe in all existing layout engines.

Even if you assume that an iframe has no intrinsic width, it certainly has no intrinsic ratio.  So you get to the "otherwise" clause in section 10.3.2, as you point out.  So the used value still ends up 300px wide, modulo this new "fits the device" text, whatever it means.

I don't see a contradiction.  More importantly, 9.2 doesn't actually specify layout in detail; that's the job of chapter 10.

I can guarantee that iframe is in fact a replaced element, and that the 300px stuff in the CSS spec is there precisely because that's what all UAs do with unstyled iframes.
Comment 11 Boris Zbarsky [:bz] 2008-01-16 21:47:29 PST
Created attachment 297494 [details] [diff] [review]
This fixes it

This also fixes checkboxes and radios, which were also not using their intrinsic height.  I haven't done any real testing on this past checking that it in fact fixes this bug.  I assume this is OK for the various XUL stuff inheriting nsLeafFrame, but it would be nice to double-check.  It would also be nice if someone has time to create some reftests...
Comment 12 Dennis Bell 2008-01-16 21:49:31 PST
(In reply to comment #10)
> > For years, naked iframes have been rendered as 150x300
> 
> Yes.  That's the intrinsic size of an iframe in all existing layout engines.
> 
> Even if you assume that an iframe has no intrinsic width, it certainly has no
> intrinsic ratio.  So you get to the "otherwise" clause in section 10.3.2, as
> you point out.  So the used value still ends up 300px wide, modulo this new
> "fits the device" text, whatever it means.
> 
> I don't see a contradiction.  More importantly, 9.2 doesn't actually specify
> layout in detail; that's the job of chapter 10.
> 
> I can guarantee that iframe is in fact a replaced element, and that the 300px
> stuff in the CSS spec is there precisely because that's what all UAs do with
> unstyled iframes.
> 

The conflict, which I grant is subtle, is that 10.3.8 is specifying how to layout replaced elemnts *with* intrinsic dimentions.  It states that it is "similar to the previous (10.3.7), except that the element has an intrinsic width".  

Since we can establish that iframes, though they be replaced elements, have *no intrinsic width*, I suggest the logical course of action is to use 10.3.7. -- the title of section 10.3.8 is erroneous, it should be "10.3.8 Absolutely positioned, replaced elements with intrinsic width" because that's what it says in the text.

As for following 9.2, I simply meant that we should take the box offsets of positioned elements into consideration to calculate the width, as is done in 10.3.7 - sorry for the miscommunication.
Comment 13 Boris Zbarsky [:bz] 2008-01-16 21:58:53 PST
Please don't quote the whole text.  It makes things harder to read for no good reason.

The "except ..." line is informative, not normative.  You raise a good point, which is that it's confusing.  I'll raise the issue of removing it with the working group.  The title of the section is quite correct.
Comment 14 Dennis Bell 2008-01-16 22:28:44 PST
(In reply to comment #13)
Sorry about the quoting thing - I was just clicking on the reply link.

Thanks for the explanation and logic.  While I disagree with the standard's stance on rendering replaced elements by ignoring the box offsets, I respect that they are the standards we're meant to follow. People ask me why use Firefox, and I say because it's standards-compliant, so I'd be a major hypocrite if I suggested we should ignore the standard because "it's wrong' in my opinion.  

Perhaps in a future CSS standard, they may revisit the 'perceived need' to differentiate rendering based on the presence of intrinsic (constant?) dimension rather than whether the element is replaced or not.  Perhaps they'll determine that the abitrary width/height/ratio assignment is, well, arbitrary, and if it comes to that point in the decision tree, render it as in 10.3.7 instead.  

Until that day, if it ever comes, the 'work-around' for being standards-compliant is just wrap the iframe  element in a positioned div that does respond to CSS left/right and set the iframe's width to 100%.  This also has to be done for img elements, because like iframes, they're replaced elements and should no longer stretch like they did in FF2.

I'm satisfied that this has been looked into sufficiently.
Comment 15 Brian Polidoro 2008-01-17 07:15:58 PST
Mutating this bug for the height issue.  Mutating bugs are better than mutating specs. :)
Comment 16 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2008-01-22 12:04:16 PST
Sounds like this bug is basically a duplicate of bug 363675.
Comment 17 Boris Zbarsky [:bz] 2008-01-22 12:09:40 PST
Indeed.  Given that the patches are basically identical, shall we just mark this duplicate?
Comment 18 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2008-01-22 12:09:55 PST
Comment on attachment 297494 [details] [diff] [review]
This fixes it

r+sr=dbaron, although I don't see how this differs from my patch for bug 363675 that didn't work.

Please add some tests as well...
Comment 19 Boris Zbarsky [:bz] 2008-01-22 12:15:13 PST
> although I don't see how this differs from my patch for bug 363675
> that didn't work.

I don't either... This patch definitely works.

I'll try to steal the tests in bug 363675, I guess.  Though again, if someone (Brian?) is willing to cook up some reftests that would be great.  I won't have time to work on the tests until sometime next week, most likely...
Comment 20 Boris Zbarsky [:bz] 2008-01-22 12:15:56 PST
Comment on attachment 297494 [details] [diff] [review]
This fixes it

Requesting approval.  This gives us IE/Safari/Opera compat for iframe sizing in this case.
Comment 21 Boris Zbarsky [:bz] 2008-01-25 13:49:48 PST
Checked in.  I added some tests too.
Comment 22 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2008-01-25 13:57:28 PST
*** Bug 363675 has been marked as a duplicate of this bug. ***
Comment 23 Myk Melez [:myk] [@mykmelez] 2008-01-25 15:07:36 PST
I backed this out on suspicion that it regressed a reftest on Mac and Windows (although it's not clear how an image can change by -1 pixels):

REFTEST UNEXPECTED FAIL: file:///builds/slave/trunk_osx/mozilla/layout/reftests/bugs/321402-3.xul

REFTEST   IMAGE 1 (TEST): [image/png;base64 data: URL]
REFTEST   IMAGE 2 (REFERENCE): [image/png;base64 data: URL]
REFTEST number of differing pixels: -1

Mac: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1201299512.1201300654.22189.gz#err0
Windows: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1201298142.1201300512.21761.gz#err0
Comment 24 Myk Melez [:myk] [@mykmelez] 2008-01-25 15:09:23 PST
The problem showed up on Linux as well.

Linux:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1201299359.1201302229.26662.gz#err0
Comment 25 Myk Melez [:myk] [@mykmelez] 2008-01-25 16:02:20 PST
The results from the ensuing builds are coming in now, and they confirm that backing out this change fixed the regression.
Comment 26 Boris Zbarsky [:bz] 2008-01-25 20:11:51 PST
Uh...  That test hits:

###!!! ASSERTION: Someone didn't override Reflow: 'Not Reached', file ../../../mozilla/layout/generic/nsLeafFrame.cpp, line 120

I guess I'll need to hunt down and kill said someone...
Comment 27 Boris Zbarsky [:bz] 2008-01-25 20:13:28 PST
Ah, I bet the problem is some subclass that does override Reflow() but not ComputeAutoSize().
Comment 28 Boris Zbarsky [:bz] 2008-01-25 20:48:46 PST
Created attachment 299369 [details] [diff] [review]
Fix XUL stuff

Just needed to make nsLeafBoxFrame not call the new ComputeAutoSize, since it has an intrinsic width and height of 0....
Comment 29 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2008-01-26 00:37:47 PST
Comment on attachment 299369 [details] [diff] [review]
Fix XUL stuff

r+sr=dbaron
Comment 30 jag (Peter Annema) 2008-01-26 00:53:41 PST
+  NS_ASSERTION(aReflowState.ComputedWidth() != NS_UNCONSTRAINEDSIZE,
...
+  NS_ASSERTION(NS_INTRINSICSIZE != aReflowState.ComputedHeight(),

Really ... :-)
Comment 31 Boris Zbarsky [:bz] 2008-01-27 13:52:09 PST
Relanded on trunk.  It stuck this time.
Comment 32 philippe (part-time) 2008-04-11 17:46:00 PDT
*** Bug 428344 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.