Created attachment 177103 [details] Reduction .
Created attachment 177151 [details] Patch
Comment on attachment 177151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177151&action=review > Source/WebCore/ChangeLog:10 > + Percentage height "is calculated with respect to the height of the generated box's containing block." So when > + calculating the used height of a replaced element do not crawl through ancestor blocks except when traversing > + anonymous blocks. Should probably add the reference here: http://www.w3.org/TR/CSS21/visudet.html#the-height-property and also the following sentence from that section: "If the height of the containing block is not specified explicitly (i.e., it depends on content height), and this element is not absolutely positioned, the value computes to 'auto'. " Needless to say this is how the other browsers do it too: http://roberthogan.net/css/computed-image-width-with-percent-height-and-fixed-ancestor.html
Comment on attachment 177151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177151&action=review > Source/WebCore/rendering/RenderReplaced.cpp:249 > + while (cb->isAnonymousBlock()) > + cb = cb->containingBlock(); I don't think this is right: an image inside an anonymous table cell will return the wrong result. Example code: <div style="display: table; height: auto"> <img src="resources/square-blue-100x100.png"> </div> This used to return false as we would consider the anonymous table cell (which is the image's containing block) but now we will go to the table. > LayoutTests/fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor-expected.html:1 > +<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> Can we use the HTML5 doctype instead? > LayoutTests/fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor-expected.html:9 > + The square below should be 100px by 100px. Bug number + title. > LayoutTests/fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor.html:5 > + img { height:100%; } Space after the colon.
Comment on attachment 177151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177151&action=review >> Source/WebCore/rendering/RenderReplaced.cpp:249 >> + cb = cb->containingBlock(); > > I don't think this is right: an image inside an anonymous table cell will return the wrong result. > > Example code: > <div style="display: table; height: auto"> > <img src="resources/square-blue-100x100.png"> > </div> > > This used to return false as we would consider the anonymous table cell (which is the image's containing block) but now we will go to the table. Great spot, but interestingly our result for that test is wrong now though and this patch doesn't change that! There's no good reason I can see in the spec for a containing block which is a table cell to say that it does not have an auto height. This was introduced in http://trac.webkit.org/changeset/91242
Hi Nikolas, cc'ing you on this. Why does hasAutoHeightOrContainingBlockWithAutoHeight() special case table cells? This causes it to mis-render the case pointed out by Julien: <div style="display: table; height: auto"> <img src="resources/square-blue-100x100.png"> </div>
Created attachment 178613 [details] Patch
Hi Nikolas - Can you take a look at this and comment #5?
Comment on attachment 178613 [details] Patch Taking this to bug 14762 instead.
(In reply to comment #5) > Hi Nikolas, > > cc'ing you on this. Why does hasAutoHeightOrContainingBlockWithAutoHeight() special case table cells? This causes it to mis-render the case pointed out by Julien: > > <div style="display: table; height: auto"> > <img src="resources/square-blue-100x100.png"> > </div> Channeling Niko, I suspect that either (a) the table cell is treated specially in an attempt to handle svg content in table cells, where the SVG really wants a size in order to lay out (but we're broken in this respect anyway) or (b) because it's just an error that got through. Table layout is not Niko's thing. I would not place too much weight on the change he made in this regard. That is, defer to Julien.
(In reply to comment #9) > (In reply to comment #5) > > Hi Nikolas, > > > > cc'ing you on this. Why does hasAutoHeightOrContainingBlockWithAutoHeight() special case table cells? This causes it to mis-render the case pointed out by Julien: > > > > <div style="display: table; height: auto"> > > <img src="resources/square-blue-100x100.png"> > > </div> > > Channeling Niko, I suspect that either (a) the table cell is treated specially in an attempt to handle svg content in table cells, where the SVG really wants a size in order to lay out (but we're broken in this respect anyway) or (b) because it's just an error that got through. > > Table layout is not Niko's thing. I would not place too much weight on the change he made in this regard. That is, defer to Julien. I think it was (a), but not sure if I recover it correctly. We probably have a test for it, if not, please write a test to check the behavior <div style="display: table; height: auto"> <svg><rect width="100%" height="100%" fill="green"/></svg> </div> Should be enough.
Resolved by 28035 and 14762