Bug 18885 - RenderLayer::enclosingPositionedAncestor should look at transforms
Summary: RenderLayer::enclosingPositionedAncestor should look at transforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-04 11:48 PDT by Simon Fraser (smfr)
Modified: 2008-07-11 15:44 PDT (History)
2 users (show)

See Also:


Attachments
Sample with two sets of divs, one of which has a transform. (1.06 KB, text/html)
2008-05-04 11:49 PDT, Simon Fraser (smfr)
no flags Details
Testcase, but it requires visual inspection (1.19 KB, text/html)
2008-07-10 14:47 PDT, Simon Fraser (smfr)
no flags Details
Patch (1.27 KB, patch)
2008-07-10 14:49 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch, testcase, changelog (5.68 KB, patch)
2008-07-11 10:54 PDT, Simon Fraser (smfr)
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2008-05-04 11:48:57 PDT
It seems that RenderLayer::enclosingPositionedAncestor() should look for transforms when walking up the RenderLayer tree for a layer to be treated as its positioned ancestor.

I noticed this while debugging: convertToLayerCoords() would give the wrong answer if there was an intermediate layer in the tree for a transformed element which was not positioned (will attach sample).
Comment 1 Simon Fraser (smfr) 2008-05-04 11:49:35 PDT
Created attachment 20960 [details]
Sample with two sets of divs, one of which has a transform.
Comment 2 Simon Fraser (smfr) 2008-07-10 14:47:43 PDT
Created attachment 22213 [details]
Testcase, but it requires visual inspection
Comment 3 Simon Fraser (smfr) 2008-07-10 14:48:14 PDT
Filed bug 19982 on enhancing DRT to dump info about transforms.
Comment 4 Simon Fraser (smfr) 2008-07-10 14:49:22 PDT
Created attachment 22214 [details]
Patch

Patch to enclosingPositionedAncestor() to respect transforms. Also removed some bogus semicolons.
Comment 5 Dave Hyatt 2008-07-10 15:13:40 PDT
Comment on attachment 22214 [details]
Patch

Pretty sure the semicolons were needed to stop some bizarre compile error on some platform.  I tried to remove this code in an earlier patch as well and was told it was needed.
Comment 6 Dave Hyatt 2008-07-10 15:14:25 PDT
Although maybe the empty braces would be sufficient for whatever platform was complaining.  I think bdash would remember.

Comment 7 Simon Fraser (smfr) 2008-07-10 16:23:40 PDT
How about rewriting those as while() loops? The for loops make my head hurt.
Comment 8 Simon Fraser (smfr) 2008-07-10 16:37:48 PDT
bdash says that for (...) { } is OK. It's for (...); that had issues.
Comment 9 Simon Fraser (smfr) 2008-07-11 10:54:07 PDT
Created attachment 22249 [details]
Patch, testcase, changelog
Comment 10 Dave Hyatt 2008-07-11 11:34:16 PDT
Comment on attachment 22249 [details]
Patch, testcase, changelog

r=me
Comment 11 Dean Jackson 2008-07-11 15:35:20 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/transforms/transform-positioned-ancestor.html
	A	LayoutTests/platform/mac/fast/transforms/transform-positioned-ancestor-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderLayer.cpp
Committed r35138