Bug 130028 – The scrolledContentOffset method should handle the hasOverflowClip check

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Radu Stavila]

Currently, all call sites of scrolledContentOffset are performing the hasOverflowClip check. This check should be moved into the scrolledContentOffset itself.

[Radu Stavila]

Created attachment 226429 [details]
Patch

[alan]

Comment on attachment 226429 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:2512
> +    LayoutPoint offsetFromRepaintContainer = roundedLayoutPoint(transformState.mappedPoint()) - scrolledContentOffset();

Is there an intention of actual rounding here? If not I'd rather use the explicit LayoutPoint(const FloatPoint&) c'tor instead. (roundedLayoutPoint() sadly doesnt do any rounding -I'll be cleaning that up at some point)

[Radu Stavila]

Comment on attachment 226429 [details]
Patch

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

>> Source/WebCore/rendering/RenderBlock.cpp:2512
>> +    LayoutPoint offsetFromRepaintContainer = roundedLayoutPoint(transformState.mappedPoint()) - scrolledContentOffset();
> 
> Is there an intention of actual rounding here? If not I'd rather use the explicit LayoutPoint(const FloatPoint&) c'tor instead. (roundedLayoutPoint() sadly doesnt do any rounding -I'll be cleaning that up at some point)

That is part of the existing code, the change here is removing the hasOverflowClip call, which was moved to scrolledContentOffset.

[alan]

(In reply to comment #3)
> (From update of attachment 226429 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226429&action=review
> 
> >> Source/WebCore/rendering/RenderBlock.cpp:2512
> >> +    LayoutPoint offsetFromRepaintContainer = roundedLayoutPoint(transformState.mappedPoint()) - scrolledContentOffset();
> > 
> > Is there an intention of actual rounding here? If not I'd rather use the explicit LayoutPoint(const FloatPoint&) c'tor instead. (roundedLayoutPoint() sadly doesnt do any rounding -I'll be cleaning that up at some point)
> 
> That is part of the existing code, the change here is removing the hasOverflowClip call, which was moved to scrolledContentOffset.
Are you planning to fix it at some point?

[Radu Stavila]

Comment on attachment 226429 [details]
Patch

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

>>>> Source/WebCore/rendering/RenderBlock.cpp:2512
>>>> +    LayoutPoint offsetFromRepaintContainer = roundedLayoutPoint(transformState.mappedPoint()) - scrolledContentOffset();
>>> 
>>> Is there an intention of actual rounding here? If not I'd rather use the explicit LayoutPoint(const FloatPoint&) c'tor instead. (roundedLayoutPoint() sadly doesnt do any rounding -I'll be cleaning that up at some point)
>> 
>> That is part of the existing code, the change here is removing the hasOverflowClip call, which was moved to scrolledContentOffset.
> 
> Are you planning to fix it at some point?

Yes - https://bugs.webkit.org/show_bug.cgi?id=130128

[Csaba Osztrogonác]

Comment on attachment 226429 [details]
Patch

Clearing flags on attachment: 226429

Committed r165537: <http://trac.webkit.org/changeset/165537>

[Csaba Osztrogonác]

All reviewed patches have been landed.  Closing bug.