| Summary: | The scrolledContentOffset method should handle the hasOverflowClip check | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Radu Stavila <stavila> | ||||
| Component: | CSS | Assignee: | Radu Stavila <stavila> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | darin, ossy, zalan | ||||
| Priority: | P2 | Keywords: | AdobeTracked | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Radu Stavila
2014-03-10 10:35:08 PDT
Created attachment 226429 [details]
Patch
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) 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. (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? 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 Comment on attachment 226429 [details] Patch Clearing flags on attachment: 226429 Committed r165537: <http://trac.webkit.org/changeset/165537> All reviewed patches have been landed. Closing bug. |