Summary: | [CSS Regions] Hit-testing is not working properly inside scrollable regions | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Radu Stavila <stavila> | ||||
Component: | CSS | Assignee: | Radu Stavila <stavila> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, WebkitBugTracker | ||||
Priority: | P2 | Keywords: | AdobeTracked | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 57312 | ||||||
Attachments: |
|
Description
Radu Stavila
2014-03-10 04:04:41 PDT
Created attachment 226313 [details]
Patch
Comment on attachment 226313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226313&action=review > Source/WebCore/rendering/RenderLayer.cpp:6879 > + IntSize scrolledContentOffset = region->fragmentContainer().hasOverflowClip() ? region->fragmentContainer().scrolledContentOffset() : IntSize(); Why would we want to skip the call to scrolledContentOffset is the container does not handle overflow? Wouldn’t the container correctly return 0,0 in that case? This seems mysterious enough that you might want to write a comment. Comment on attachment 226313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226313&action=review >> Source/WebCore/rendering/RenderLayer.cpp:6879 >> + IntSize scrolledContentOffset = region->fragmentContainer().hasOverflowClip() ? region->fragmentContainer().scrolledContentOffset() : IntSize(); > > Why would we want to skip the call to scrolledContentOffset is the container does not handle overflow? Wouldn’t the container correctly return 0,0 in that case? This seems mysterious enough that you might want to write a comment. The first line of the RenderBox::scrolledContentOffset() method is ASSERT(hasOverflowClip()); Comment on attachment 226313 [details]
Patch
r=me with the informative comment Darin asked for.
After discussing with Antti on IRC, we decided to not add the comment, since throughout the code, every time the scrolledContentOffset is used, it's preceeded by the hasOverflowClip check. I will submit an issue to move the check inside the scrolledContentOffset method itself. Comment on attachment 226313 [details] Patch Clearing flags on attachment: 226313 Committed r165388: <http://trac.webkit.org/changeset/165388> All reviewed patches have been landed. Closing bug. |