Bug 58006 - Images with percentage based height/max-height are missing when they are inside blocks inside tables
Summary: Images with percentage based height/max-height are missing when they are insi...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on: 15359
  Show dependency treegraph
Reported: 2011-04-06 19:02 PDT by Adele Peterson
Modified: 2011-05-02 13:49 PDT (History)
4 users (show)

See Also:

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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adele Peterson 2011-04-06 19:02:39 PDT
Created attachment 88557 [details]

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.

Comment 1 Adele Peterson 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.
Comment 2 Adele Peterson 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.
Comment 3 Adele Peterson 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.
Comment 4 Adele Peterson 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

Fixed with:
Comment 5 Adele Peterson 2011-04-12 22:04:17 PDT
This also occurs with percentage-based height.  Attaching a new test case.
Comment 6 Adele Peterson 2011-04-12 22:04:42 PDT
Created attachment 89338 [details]
New test
Comment 7 Adele Peterson 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.
Comment 8 Adele Peterson 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.
Comment 9 mitz 2011-05-02 11:28:43 PDT
Created attachment 91945 [details]
Extend the fix for bug 15359 to cover the nested case
Comment 10 mitz 2011-05-02 11:51:46 PDT
Fixed in r85499. <http://trac.webkit.org/changeset/85499>