| Summary: | RenderLayer::backgroundClipRect should not check parent() | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> | ||||
| Component: | Layout and Rendering | Assignee: | Julien Chaffraix <jchaffraix> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | simon.fraser, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Julien Chaffraix
2011-12-02 19:15:55 PST
Created attachment 117734 [details]
Proposed change 1: remove parent() + some clean-up.
Comment on attachment 117734 [details] Proposed change 1: remove parent() + some clean-up. View in context: https://bugs.webkit.org/attachment.cgi?id=117734&action=review Did you do archaeology to see why the parent() check was there? > Source/WebCore/rendering/RenderLayer.cpp:3617 > + if (position == AbsolutePosition) > + return parentRects.posClipRect(); Does renderer-> isPositioned() return true for position:relative? If so, then you broke stuff. Comment on attachment 117734 [details] Proposed change 1: remove parent() + some clean-up. View in context: https://bugs.webkit.org/attachment.cgi?id=117734&action=review >> Source/WebCore/rendering/RenderLayer.cpp:3617 >> + return parentRects.posClipRect(); > > Does renderer-> isPositioned() return true for position:relative? If so, then you broke stuff. No, it does not. Here is the comment in RenderObject (still accurate per RenderBox::updateBoxModelInfoFromStyle): bool isPositioned() const { return m_positioned; } // absolute or fixed positioning isPositioned() also returns true for the RenderView but it will never be called here as it has no parent. (In reply to comment #2) > (From update of attachment 117734 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117734&action=review > > Did you do archaeology to see why the parent() check was there? I just did as I missed your question. The parent() check inside backgroundClipRect is a legacy of bug 29346 which split calculateRects in 2. It looks like it was missed during the split that all call sites were already checking parent() which made the check redundant. Comment on attachment 117734 [details] Proposed change 1: remove parent() + some clean-up. Clearing flags on attachment: 117734 Committed r103955: <http://trac.webkit.org/changeset/103955> All reviewed patches have been landed. Closing bug. |