Bug 85581 - Percentage height replaced elements sometimes cause overflow of table contents
Summary: Percentage height replaced elements sometimes cause overflow of table contents
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Terry Anderson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-03 22:32 PDT by Terry Anderson
Modified: 2012-05-18 14:41 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.89 KB, patch)
2012-05-03 23:06 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff
Patch (64.08 KB, patch)
2012-05-11 13:45 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff
Patch (5.56 KB, patch)
2012-05-18 12:04 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Terry Anderson 2012-05-03 22:32:11 PDT
Reported here: http://code.google.com/p/chromium/issues/detail?id=122806

To view an example of the bug, in chromium visit http://support.google.com/ics/nexus/bin/answer.py?hl=en&answer=1637532 and you will notice that the Settings icons are much bigger than intended.

The image has a percentage height of 60%. From inspecting the page, it appears that this is relative to the <table> rather than to its parent <p>, but the CSS specs say that the containing block of this image should in fact be the <p> element.
Comment 1 Terry Anderson 2012-05-03 23:06:02 PDT
Created attachment 140163 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-03 23:09:25 PDT
Attachment 140163 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Eric Seidel (no email) 2012-05-03 23:09:45 PDT
Comment on attachment 140163 [details]
Patch

This needs tests.  It's also certainly not chromium only.
Comment 4 Terry Anderson 2012-05-11 13:45:25 PDT
Created attachment 141489 [details]
Patch
Comment 5 Terry Anderson 2012-05-11 13:55:56 PDT
(In reply to comment #4)
> Created an attachment (id=141489) [details]
> Patch

I created the layout test fast/replaced/table-replaced-element.html is a scaled-down version of the page where the bug was originally found (http://support.google.com/ics/nexus/bin/answer.py?hl=en&answer=1637532).

If you view fast/replaced/table-replaced-element.html in chromium, you will see that the presence of the percent height image causes an overflow of the table contents. This patch fixes the overflow problem, but does not respect the 60% height attribute of the image.

(I have re-named this bug from "Percentage height replaced elements are sometimes displaying with incorrect size" to "Percentage height replaced elements sometimes cause overflow of table contents")
Comment 6 Eric Seidel (no email) 2012-05-15 13:48:55 PDT
Comment on attachment 141489 [details]
Patch

I'm happy to reivew this.  But why do we need the wall of text?  And does this need to be a pixel test?  Can we use a dumpAsText or reference test instead?  Can we use a div/divs with fixed heights instead of the wall fo text?  inline-blocks will flow like text and you can control their size.

r- because the test could be much better.  Otherwise the chagne looks OK.
Comment 7 Terry Anderson 2012-05-18 12:04:48 PDT
Created attachment 142759 [details]
Patch
Comment 8 Eric Seidel (no email) 2012-05-18 14:24:41 PDT
Comment on attachment 142759 [details]
Patch

Thanks.
Comment 9 WebKit Review Bot 2012-05-18 14:41:47 PDT
Comment on attachment 142759 [details]
Patch

Clearing flags on attachment: 142759

Committed r117633: <http://trac.webkit.org/changeset/117633>
Comment 10 WebKit Review Bot 2012-05-18 14:41:52 PDT
All reviewed patches have been landed.  Closing bug.