Closed Bug 729143 Opened 12 years ago Closed 12 years ago

div with height=100% and overflow renders incorrectly

Categories

(Core :: Layout: Tables, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: aaron, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; .NET4.0C; .NET4.0E; OfficeLiveConnector.1.5; OfficeLivePatch.1.3)

Steps to reproduce:

Overflow doesn't seem work properly any longer on divs inside table cells in Firefox v10. 

The test case below should have produced a ~100 x 100px table with a div inside it with scrollbars:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html>
<body>
 <table cellspacing="0" cellpadding="0" border="0" style="width: 100px; height: 100px; border: 1px solid black;">
  <tbody style="height: 100%;"> <!-- required for older ffox versions -->
   <tr>
    <td style="height: 100%;">
     <div style="border: solid 1px red; height: 100%; width: 100%;overflow:scroll;">
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
      row<br />
     </div>
    </td>
   </tr>
  </tbody>
 </table>
</body>
</html>


Actual results:

The div does not overflow and simply shows the entire contents. The table grows accordingly because of this.

Tried different DOCTYPES but it had no effect.


Expected results:

The div should have been as large as the table, 100x100px. 
Worked in previous versions of Firefox.

Renders properly in Chrome, Safari, IE7+ and Firefox 9 and below.
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/60e86b847759
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20110929 Firefox/10.0a1 ID:20110929122038
Fails:
http://hg.mozilla.org/mozilla-central/rev/af3668a89015
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20110929 Firefox/10.0a1 ID:20110929141938
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=60e86b847759&tochange=af3668a89015


In local build
Last Good : 34f184d2a6f8
First Bad : 00f422b2cf36

Triggered by:
00f422b2cf36	Ehsan Akhgari — Bug 10209 - Part 6: Implement the CSS "containing block" concept correctly as a binary relation, as opposed to a unary relation; r=bzbarsky
Blocks: 10209
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: Tables
Ever confirmed: true
Product: Firefox → Core
QA Contact: untriaged → layout.tables
Attached file Reporter's HTML
Keywords: regression
The <tr> has auto height and is being treated as the containing block for the cell.

I assume it didn't use to be considered a containing block?  Should we reinstate that (so skip table-row frames in GetNearestBlockContainer)?
In the interim, two notes:

1)  "properly" is unclear here; the spec doesn't actually define the behavior of this
    markup.  I do think we should restore the old behavior, though.
2)  A workaround for the moment is to set "height: 100%" on the <tr>.
Attachment #599458 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Version: 10 Branch → Trunk
So what situations is this changing behavior in?  And is it just reverting to behavior we used to have?  (Why did we change it?)
It certainly changes the behavior of percentage heights on table cells.  They are now resolved against the table row group, not the table row, which importantly means that they no longer compute to auto if the row group has non-auto height.

This used to be the case before as well: table row groups returned true from IsContainingBlock(), but table rows did not. I _think_ this should just revert to the behavior we used to have, since the only time we'd get to this code with table row is when looking up the containing block for a table cell.

I believe the behavior change was inadvertent, but I'm not 100% sure on that.  I did start a try run with this change to see whether anything pops up.
Reftests, at least on Linux, seem to be green.
(The patch you posted to this bug doesn't have the reftest, although it sounds like it made it into a later version of the patch.)
Attached patch With testSplinter Review
Oops, yes.  The reftest somehow ended up in the try-syntax changeset...  Fixed now.
Attachment #599486 - Flags: review?(dbaron)
Attachment #599458 - Attachment is obsolete: true
Attachment #599458 - Flags: review?(dbaron)
Comment on attachment 599486 [details] [diff] [review]
With test

ok, r=dbaron
Attachment #599486 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca7abbb11f4a
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/ca7abbb11f4a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
See Also: → 1421660
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: