Bug 58006

Summary: Images with percentage based height/max-height are missing when they are inside blocks inside tables
Product: WebKit Reporter: Adele Peterson <adele>
Component: TablesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, ddkilzer, hyatt, mitz
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 15359    
Bug Blocks:    
Attachments:
Description Flags
test
none
New test
none
Extend the fix for bug 15359 to cover the nested case simon.fraser: review+

Adele Peterson
Reported 2011-04-06 19:02:39 PDT
Created attachment 88557 [details] test Images with max-height:100% are missing when they are inside blocks inside tables See attached test case. It works fine in Firefox and IE. I think this has something to do with this code in RenderBox::availableLogicalHeightUsing if (isTableCell() && (h.isAuto() || h.isPercent())) return overrideSize() - borderAndPaddingLogicalWidth(); In this case, if you check hasOverrideSize() you get false, and overrideSize just returns -1. So I think we need some kind of overrideLogical… method that's like the overrideHeight and overrideWidth methods that will check hasOverrideSize for you. <rdar://problem/7972529>
Attachments
test (321 bytes, text/html)
2011-04-06 19:02 PDT, Adele Peterson 		no flags
Details
New test (1.08 KB, text/html)
2011-04-12 22:04 PDT, Adele Peterson 		no flags
Details
Extend the fix for bug 15359 to cover the nested case (5.70 KB, patch)
2011-05-02 11:28 PDT, mitz 		simon.fraser: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adele Peterson
Comment 1 2011-04-06 19:24:41 PDT
As a test, I just added a check of hasOverrideSize() to that condition, and it gets the image to the right size. That is probably overly simplistic though.
Adele Peterson
Comment 2 2011-04-06 19:29:39 PDT
That experiment breaks tables/mozilla/bugs/bug137388-1.html so I'll use that test to understand how this code is supposed to work.
Adele Peterson
Comment 3 2011-04-11 22:28:27 PDT
One thing that puzzles me is that the inner paragraph element is getting the right height. If I set the exact same height on the paragraph explicitly, then the image also gets the right size.
Adele Peterson
Comment 4 2011-04-11 23:16:20 PDT
Sounds a bit similar to this old bug: JPEG image not shown when height is specified as percentage inside a table https://bugs.webkit.org/show_bug.cgi?id=15359 Fixed with: http://trac.webkit.org/changeset/29039
Adele Peterson
Comment 5 2011-04-12 22:04:17 PDT
This also occurs with percentage-based height. Attaching a new test case.
Adele Peterson
Comment 6 2011-04-12 22:04:42 PDT
Created attachment 89338 [details] New test
Adele Peterson
Comment 7 2011-04-12 23:06:41 PDT
Now I'm thinking that the reason RenderTableSection's cell children flex code doesn't run is that percentHeightDescendants doesn't ever include this image element. In http://trac.webkit.org/changeset/36513, Dan wrote this comment about code in RenderBox that adds to that list: (WebCore::RenderBox::calcReplacedHeightUsing): Changed to skip over anonymous containing blocks, if any, and when that happens, use addPercentHeightDescendant() to ensure that the non-anonymous block is aware of the dependent percent-height box. In this case, because the containing block is never an anonymous block, we never add the image element to any RenderBox's list. I tried an experiment in RenderBox::computeReplacedLogicalHeightUsing to change that loop to a do while loop so the first containing block calls addPercentHeightDescendant even if its not an anonymous block. Again, this experiment fixed the original problem -- and broke the last test case in my attachment horribly. So maybe I'm off in the weeds here.
Adele Peterson
Comment 8 2011-04-13 12:07:30 PDT
That experiment was a bust. Adding to percentHeightDescendant had no effect, but screwing up the concept of what is the containing block did.
mitz
Comment 9 2011-05-02 11:28:43 PDT
Created attachment 91945 [details] Extend the fix for bug 15359 to cover the nested case
mitz
Comment 10 2011-05-02 11:51:46 PDT
Fixed in r85499. <http://trac.webkit.org/changeset/85499>
Note You need to log in before you can comment on or make changes to this bug.