Bug 14749

Summary: percentage top value on position:relative child not calculated correctly if parent has percentage height
Product: WebKit Reporter: Andrew Stibbard <xhva.net>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: browserbugs2, ddkilzer, hyatt, mh+webkit, mitz, sam
Priority: P2 Keywords: HasReduction, InRadar, ReviewedForRadar
Version: 523.x (Safari 3)   
Hardware: PC   
OS: All   
URL: http://xhva.net/index.html
Attachments:
Description Flags
Reduced testcase for percentage height problem
none
Reduced testcase for percentage top and percentage height conflict
none
Reduced testcase for percentage top and percentage height conflict (content divs)
none
Patch mitz: review+

Description Andrew Stibbard 2007-07-24 08:37:53 PDT
Percentage heights on the html and body elements calculate correctly as demonstrated on PPK's old demo page:

http://www.xs4all.nl/~ppk/css2tests/height/heightdocbodyhtml.html

However, when a child is given position:relative, both percentages seem to be ignored.

I'm using relative positioning and negative margins to achieve vertical centering at the provided URL.

Tested on build 522.13.1 -- reduced testcase to follow.

I also tried min-height on html and body as a workaround to at least show the entire div at the top of the viewport, but min-height has no effect unless a pixel value is supplied for height. Should I file a separate bug for that?
Comment 1 Andrew Stibbard 2007-07-24 08:42:15 PDT
Created attachment 15665 [details]
Reduced testcase for percentage height problem

Displays correctly in Firefox 2 and IE 6. Body element doesn't expand in Safari 522.13.1.
Comment 2 David Kilzer (:ddkilzer) 2007-07-24 08:45:25 PDT
(In reply to comment #0)
> I also tried min-height on html and body as a workaround to at least show the
> entire div at the top of the viewport, but min-height has no effect unless a
> pixel value is supplied for height. Should I file a separate bug for that?

Please do.  Thanks!

Comment 3 David Kilzer (:ddkilzer) 2007-07-24 08:48:07 PDT
Verified with a local debug build of WebKit r24562 with Safari 3.0.2 (522.12) on Mac OS X 10.4.10 (8R218).

Works correctly in Firefox 2.0.0.5 and Opera 9.21.

Not a regression as Safari 2 with original WebKit renders the same way.

Comment 4 Andrew Stibbard 2007-07-25 01:41:56 PDT
While working on the min-height testcase, I realised the real issue here is the top:50% not taking effect when the parent's height is a percentage. If I change either of those percentages to PX values, the problem goes away.

The html and body elements are sized correctly, even in the case of min-height. I've changed the bug summary to reflect that.

(Using just min-height with a PX value doesn't make top:50% calculate correctly either. I'll file a new bug for that.)

Better testcases to follow. Thanks for your immediate response David :)
Comment 5 Andrew Stibbard 2007-07-25 01:44:25 PDT
Created attachment 15677 [details]
Reduced testcase for percentage top and percentage height conflict

Shows the html and body elements (blue and green) sized correctly, but the percentage top value not taking effect.
Comment 6 Andrew Stibbard 2007-07-25 01:46:47 PDT
Created attachment 15678 [details]
Reduced testcase for percentage top and percentage height conflict (content divs)

Testcase showing the percentage conflict with content divs.
Comment 7 Andrew Stibbard 2007-07-25 01:50:45 PDT
Comment on attachment 15677 [details]
Reduced testcase for percentage top and percentage height conflict

arg, attached first testcase as text/plain instead of text/html. Sorry for the bugspam.
Comment 8 Andrew Stibbard 2007-07-25 03:12:57 PDT
Filed Bug 14762 detailing the min-height issues.
Comment 9 David Kilzer (:ddkilzer) 2007-12-18 05:47:39 PST
Bug 15930 may be a duplicate of this bug.

Comment 10 Beth Dakin 2009-03-11 16:13:47 PDT
Created attachment 28501 [details]
Patch

Hyatt and I talked about this change on IRC. He felt a bit nervous about it because he had a vague feeling that there was some reason we originally included the if-statements that I remove with this patch, but he couldn't quite remember what it what. Here is what I can tell potential reviewers about the potential validity of this patch:

-The new behavior matches Firefox and IE with the given test cases. 
-All LayoutTests pass.
-I searched through the history of these lines of code with the aid of trac's handy annotate feature. The if-statements were added in revision 4! So I take it kocienda was just merging them over from khtml.
Comment 11 Beth Dakin 2009-03-11 16:26:03 PDT
Fixed with revision 41602.
Comment 12 David Kilzer (:ddkilzer) 2009-03-11 21:25:47 PDT
<rdar://problem/6172925>
Comment 13 Mike Hommey 2009-03-26 23:26:15 PDT
*** Bug 19930 has been marked as a duplicate of this bug. ***
Comment 14 Gérard Talbot 2009-06-14 14:28:48 PDT
Hello all,

There may be a problem with this bug report and/or the patch for this bug. 

When the height of the containing block of a child or descendant is not specified explicitly (i.e., it depends on content height), and this element is not absolutely positioned, [then] the [height] value computes to 'auto' according to section 10.5 of latest CSS 2.1 spec. 

height: auto is *_not_* height: 100% for [typical] containing block elements like <body> element or the <html> root element.

http://www.w3.org/TR/CSS21/visudet.html#the-height-property

height: auto makes the height entirely dependent and defined by the intrinsec content. height: 100% for the root element refers to the initial containing block which is the browser window viewport. The former can not be computed in advance; the latter can be computed in advance.

Right now, Safari 4.0 build 530.17 fails this test:
http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/PercentualRelativePositioning.html

A percentage height (or percentage top offset value) specified on a child or descendant must be resolved as auto if its containing block's height is auto, if its containing block's height is not specified explicitly (i.e., if its height depends on its intrinsec content).

Parent is also not the same as containing block. Containing block is not necessarly the parent.

regards, Gérard

Comment 15 mitz@webkit.org 2009-06-14 14:48:33 PDT
(In reply to comment #14)
> Right now, Safari 4.0 build 530.17 fails this test:
> http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/PercentualRelativePositioning.html

Can you please file a new bug about this apparent regression? Thanks!
Comment 16 Gérard Talbot 2009-06-14 20:41:53 PDT
Mitz, I just filed bug 26396.

regards, Gérard