It does work in IE 7+ and Opera. It also doesn't work in Firefox. http://elv1s.ru/files/html+css/max-height-within-table.png
It works with FireFox 3.6.1. I think it should be fixed. According to CSS2.1 (http://www.w3.org/TR/CSS2/visudet.html#propdef-height), the percentage height of html box (in this case, height:100%) is relative to the initial containing block.
Created attachment 157531 [details] Patch
Comment on attachment 157531 [details] Patch Attachment 157531 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13455998 New failing tests: fast/replaced/table-percent-height.html
Created attachment 157562 [details] Archive of layout-test-results from gce-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
It turns out the behaviour in the reduction is undefined by CSS. From http://lists.w3.org/Archives/Public/public-css-testsuite/2012Aug/0029.html: " http://roberthogan.net/css/max-height-percentage-replaced-006.htm html, body, table, tbody, td { width: 100%; height: 100%; padding: 0; margin: 0; vertical-align: top; } table-row-groups can not be set a height; that's given by the spec or http://wiki.csswg.org/spec/css2.1#issue-133 and table-row-groups can not be set to a width 10.2 Content width: the 'width' property 'width' Applies to: all elements but non-replaced inline elements, table rows, and row groups http://www.w3.org/TR/CSS21/visudet.html#the-width-property So, we have again a situation where any size is possible, permissible."
Created attachment 157869 [details] Patch
Comment on attachment 157869 [details] Patch Attachment 157869 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13477569 New failing tests: fast/css/max-height-percentage-replaced-006.htm
Created attachment 157871 [details] Archive of layout-test-results from gce-cr-linux-06 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-06 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 157873 [details] Patch
Comment on attachment 157873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157873&action=review > Source/WebCore/ChangeLog:63 > + The containing block for a table cell is undefined by CSS, as is the height of table row groups, so in reality there is no Are you sure the containing block of a table cell is undefined? Here is my interpretation: Section 10.1 (definition of containing block) "For other elements [non-root elements], if the element’s position is ’relative’ or ’static’, the containing block is formed by the content edge of the nearest block container ancestor box." Section 17.4 Tables in the visual formatting model "The table wrapper box is a ’block’ box if the table is block-level, and an ’inline-block’ box if the table is inline-level. The table wrapper box establishes a block formatting context." As none of the other table parts are blocks, the cell's containing block is the table wrapper box. > Source/WebCore/rendering/RenderBox.cpp:2253 > // FIXME: This needs to be made block-flow-aware. If the cell and image are perpendicular block-flows, this isn't right. You should probably rephrase this. block-flows was renamed to writing-modes since this was written. > Source/WebCore/rendering/RenderBox.cpp:-2260 > - // <http://bugs.webkit.org/show_bug.cgi?id=15359> Please mention somewhere in the ChangeLog that bug 15359 is solved so this is a rotten comment. Btw, bug 15359 mentions bug 81084. Shouldn't this change also solve bug 81084? (if it does, it should be mentioned in the ChangeLog). > Source/WebCore/rendering/RenderBox.cpp:2265 > - return valueForLength(logicalHeight, availableHeight - borderAndPaddingLogicalHeight()); > + return availableHeight - borderAndPaddingLogicalHeight(); This will break the case of -webkit-calc when there is no circular dependency (ie -webkit-calc(50px + 25px) for example). Not sure how much we care about this case but it's worth mentioning it.
CC'ing Mike in case he has a comment about the -webkit-calc change.
(In reply to comment #10) > (From update of attachment 157873 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=157873&action=review > > > Source/WebCore/ChangeLog:63 > > + The containing block for a table cell is undefined by CSS, as is the height of table row groups, so in reality there is no > > Are you sure the containing block of a table cell is undefined? > > Here is my interpretation: > > Section 10.1 (definition of containing block) > > "For other elements [non-root elements], if the element’s position is ’relative’ or ’static’, the containing > block is formed by the content edge of the nearest block container ancestor box." > > Section 17.4 Tables in the visual formatting model > > "The table wrapper box is a ’block’ box if the table is block-level, and an > ’inline-block’ box if the table is inline-level. The table wrapper box establishes a block > formatting context." > > As none of the other table parts are blocks, the cell's containing block is the table wrapper box. I took this to the css-test-suite list when I was looking at the issue: http://lists.w3.org/Archives/Public/public-css-testsuite/2012Aug/0023.html The consensus there at the time was that it probably does not. They agreed with you that the only possible intepretation was the table-wrapper box - but no one treats it that way so it isn't a sane interpreation. :/ > > > Source/WebCore/rendering/RenderBox.cpp:2253 > > // FIXME: This needs to be made block-flow-aware. If the cell and image are perpendicular block-flows, this isn't right. > > You should probably rephrase this. block-flows was renamed to writing-modes since this was written. > > > Source/WebCore/rendering/RenderBox.cpp:-2260 > > - // <http://bugs.webkit.org/show_bug.cgi?id=15359> > > Please mention somewhere in the ChangeLog that bug 15359 is solved so this is a rotten comment. > > Btw, bug 15359 mentions bug 81084. Shouldn't this change also solve bug 81084? (if it does, it should be mentioned in the ChangeLog). I'll take a look. > > > Source/WebCore/rendering/RenderBox.cpp:2265 > > - return valueForLength(logicalHeight, availableHeight - borderAndPaddingLogicalHeight()); > > + return availableHeight - borderAndPaddingLogicalHeight(); > > This will break the case of -webkit-calc when there is no circular dependency (ie -webkit-calc(50px + 25px) for example). Not sure how much we care about this case but it's worth mentioning it.
> > Source/WebCore/rendering/RenderBox.cpp:2265 > > - return valueForLength(logicalHeight, availableHeight - borderAndPaddingLogicalHeight()); > > + return availableHeight - borderAndPaddingLogicalHeight(); > > This will break the case of -webkit-calc when there is no circular dependency (ie -webkit-calc(50px + 25px) for example). Not sure how much we care about this case but it's worth mentioning it. I would of course like to have calc() work for tables. However, there is a get-out-of-jail-free clause in the calc spec that allows ignoring calc for tables.
Comment on attachment 157873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157873&action=review As mentioned on IRC, the test cases should really be added to the official test suite as we are breaking some compatibility here. Also if you don't intent to upload them to the test suite, they could probably be written in a better way (like HTML 5 doctype, maybe some would be check-layout.js, ...) >>> Source/WebCore/ChangeLog:63 >>> + The containing block for a table cell is undefined by CSS, as is the height of table row groups, so in reality there is no >> >> Are you sure the containing block of a table cell is undefined? >> >> Here is my interpretation: >> >> Section 10.1 (definition of containing block) >> >> "For other elements [non-root elements], if the element’s position is ’relative’ or ’static’, the containing >> block is formed by the content edge of the nearest block container ancestor box." >> >> Section 17.4 Tables in the visual formatting model >> >> "The table wrapper box is a ’block’ box if the table is block-level, and an >> ’inline-block’ box if the table is inline-level. The table wrapper box establishes a block >> formatting context." >> >> As none of the other table parts are blocks, the cell's containing block is the table wrapper box. > > I took this to the css-test-suite list when I was looking at the issue: http://lists.w3.org/Archives/Public/public-css-testsuite/2012Aug/0023.html > > The consensus there at the time was that it probably does not. They agreed with you that the only possible intepretation was the table-wrapper box - but no one treats it that way so it isn't a sane interpreation. :/ OK, I was just picking you on the fact that you said 'there is no defined containing block', which there is even if it doesn't make sense. I think you should update the prose to reflect that there *is* a definition in the spec but all browsers ignore it. >>> Source/WebCore/rendering/RenderBox.cpp:2265 >>> + return availableHeight - borderAndPaddingLogicalHeight(); >> >> This will break the case of -webkit-calc when there is no circular dependency (ie -webkit-calc(50px + 25px) for example). Not sure how much we care about this case but it's worth mentioning it. > > As Mike pointed out, this is still an issue but something we can move to a follow-up bug.
Created attachment 180601 [details] Patch
Comment on attachment 180601 [details] Patch Attachment 180601 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15446832 New failing tests: tables/mozilla/bugs/bug131020.html fast/replaced/percent-height-in-anonymous-block-in-table.html fast/table/padding-height-and-override-height.html
Created attachment 180604 [details] Patch
Comment on attachment 180604 [details] Patch Attachment 180604 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15458759 New failing tests: fast/css/max-height-percentage-replaced-003.htm fast/css/max-height-percentage-replaced-001.htm fast/css/max-height-percentage-replaced-002.htm fast/replaced/table-percent-height.html fast/css/min-height-percentage-replaced-001.htm fast/css/min-height-percentage-replaced-002.htm fast/css/max-height-percentage-replaced-004.htm
Comment on attachment 180604 [details] Patch Attachment 180604 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15884975 New failing tests: fast/css/max-height-percentage-replaced-003.htm fast/css/max-height-percentage-replaced-001.htm fast/css/max-height-percentage-replaced-002.htm fast/replaced/table-percent-height.html fast/css/min-height-percentage-replaced-001.htm fast/css/min-height-percentage-replaced-002.htm fast/css/max-height-percentage-replaced-004.htm
Created attachment 187424 [details] Patch
Comment on attachment 187424 [details] Patch Attachment 187424 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16474354 New failing tests: fast/replaced/table-percent-height.html
Comment on attachment 187424 [details] Patch Attachment 187424 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16465562 New failing tests: fast/replaced/table-percent-height.html
Comment on attachment 187424 [details] Patch Attachment 187424 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16465674 New failing tests: http/tests/cache/cached-main-resource.html fast/replaced/table-percent-height.html
Comment on attachment 187424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187424&action=review Also there seem to be a fair amount of behavioral change in our tests that will probably cause some compatibility issues (I have been CC'ed on more regressions that I would have liked lately). Thus the bar to us changing should be higher and I would prefer if the tests (at least the 2 cores) were accepted into the official CSS 2.1 test suite as that would ensure our compliance. > LayoutTests/fast/css/min-height-percentage-replaced-001.htm:7 > + <link rel="author" title="WebKit" href="http://www.webkit.org/"> > + <link rel="author" title="Robert Hogan" href="robert@webkit.org"> > + <link rel="help" href="http://www.w3.org/TR/CSS21/visudet.html#min-max-heights"> If you don't intent to submit these to the W3C these entries are just useless and I find them confusing to be frank. However as you seem to be changing our behavior (and breaking table-percent-height.html), I would rather see them accepted first, especially: * fast/css/height-percentage-replaced-004.htm * fast/css/max-height-percentage-replaced-003.htm as they represents the core of the change. > LayoutTests/platform/chromium-win/fast/replaced/table-percent-height-expected.txt:71 > +FAIL getHeight('input-checkbox-75') [4.5px] is not 75% of getHeight('input-checkbox-100') [6px]. > PASS getWidth('input-file-75') is getWidth('input-file-100') > PASS getHeight('input-file-75') != '0px' is true > -PASS getHeight('input-file-75') is 75% of getHeight('input-file-100'). > -PASS getWidth('input-image-75') is '75px' > -PASS getHeight('input-image-75') is '75px' > +FAIL getHeight('input-file-75') [16.5px] is not 75% of getHeight('input-file-100') [22px]. > +PASS getWidth('input-image-75') is '100px' > +PASS getHeight('input-image-75') is '100px' > PASS getWidth('input-image-100') is '100px' > PASS getHeight('input-image-100') is '100px' > PASS getWidth('input-radio-75') is getWidth('input-radio-100') > PASS getHeight('input-radio-75') != '0px' is true > -PASS getHeight('input-radio-75') is 75% of getHeight('input-radio-100'). > +FAIL getHeight('input-radio-75') [2.25px] is not 75% of getHeight('input-radio-100') [3px]. FAIL :( Is there any way you can make them say PASS as it's a no-go from my perspective to have a passing test say fail.
There is also an issue in my site. It turns out the behaviour in the reduction is undefined by CSS. https://centralmenus.com/css/max-height-percentage-replaced-001.htm html, body, table, tbody, td { width: 100%; height: 100%; padding: 0; margin: 0; vertical-align: top; } table-row-groups can not be set a height; that's given by the spec or