percentage top value of position:relative element not calculated using parent's min-height unless height set

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andrew Stibbard]

A position:relative element with a percentage top value is calculated incorrectly if its parent only has min-height set. Applying a non-percentage height to the parent, or changing top to px fixes the calc.

Tested on build 522.13.1. Firefox 2 also has this issue. Please see Bug 14749 for a related issue with percentages.

Testcase to follow.

[Andrew Stibbard]

Created attachment 15679 [details]
Reduced testcase demonstrating min-height and relative top issues

Testcase demonstrating issues with min-height and percentage top positioning.

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> Created an attachment (id=15679) [edit]
> Reduced testcase demonstrating min-height and relative top issues

The first two boxes and the last box should be 50% green on top, and 50% green on bottom.  (The larger one in the middle is green/blue/green, but not evenly distributed.)

Opera 9.21 apparently gets all of these test cases correct.

This is not a regression as Safari 2.0.4 (419.3) with original WebKit on Mac OS X 10.4.10 (8R218) behaves the same way.  Tested with a local debug build of WebKit r24586 with Safari 3 Public Beta v. 3.0.2 (522.12) on 10.4.10.

[Robert Hogan]

Created attachment 184298 [details]
Patch

[WebKit Review Bot]

Comment on attachment 184298 [details]
Patch

Attachment 184298 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16084301

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

[Julien Chaffraix]

Comment on attachment 184298 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184298&action=review

The change makes sense, some comments. There were some questions in bug 103812 that may be good to clear out here too.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:394
> +    while (cb->isAnonymousBlock() && !cb->isTableCell())
> +        cb = cb->containingBlock();

If you hit an anonymous cell, you are done as it *cannot* have anything other that the initial values for logical height, top and bottom.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:396
> +    if (!cb->style()->logicalHeight().isAuto() || (!cb->style()->top().isAuto() && !cb->style()->bottom().isAuto()))

This line looks wrong (and was wrong before you moved it here) as we check the *logical* height but the *physical* top and bottom.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:428
> -        && (!containingBlock->style()->height().isAuto()
> +        && (!containingBlock->hasAutoHeightOrContainingBlockWithAutoHeight()

Note that this replaced height() with logicalHeight() in this check which means that we should probably have a test for vertical writing modes.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:434
> -        && (!containingBlock->style()->height().isAuto()
> +        && (!containingBlock->hasAutoHeightOrContainingBlockWithAutoHeight()

Ditto.

> LayoutTests/fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor.html:13
> +    <!-- In strict mode, an image with height set to 100% and a fixed height ancestor should expand to its instrinsic height and width. -->
> +    <!-- Percentage height "is calculated with respect to the height of the generated box's containing block." --> 

This is a super useful explanation to dump in your output.

[Robert Hogan]

Created attachment 187140 [details]
Patch

[Julien Chaffraix]

Comment on attachment 187140 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187140&action=review

> LayoutTests/ChangeLog:13
> +        * fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor-expected.html: Added.
> +        * fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor.html: Added.
> +        * fast/replaced/computed-image-width-with-percent-height-inside-table-cell-and-fixed-ancestor-expected.html: Added.
> +        * fast/replaced/computed-image-width-with-percent-height-inside-table-cell-and-fixed-ancestor.html: Added.

Could it be possible to have vertical-writing mode version of these tests?

> LayoutTests/fast/block/percent-top-parent-respects-min-height.html:39
> +        <script>
> +            checkLayout('.box')
> +        </script>

You can put that on an onload listener:

<body onload="checkLayout('.box')">

> LayoutTests/fast/replaced/computed-image-width-with-percent-height-and-fixed-ancestor-expected.html:19
> +

Stray empty line (present on all the following files but not repeated).

[Robert Hogan]

Committed r143102: <http://trac.webkit.org/changeset/143102>

[Alexey Proskuryakov]

This caused bug 113077, bug 113526 and bug 115090.