Bug 130015 - [CSS Regions] Hit-testing is not working properly inside scrollable regions
Summary: [CSS Regions] Hit-testing is not working properly inside scrollable regions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Radu Stavila
URL:
Keywords: AdobeTracked
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2014-03-10 04:04 PDT by Radu Stavila
Modified: 2014-03-10 11:43 PDT (History)
7 users (show)

See Also:


Attachments
Patch (9.31 KB, patch)
2014-03-10 10:05 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Radu Stavila 2014-03-10 04:04:41 PDT
Hit-testing on elements inside scrollable regions does not take the region's scroll offset into consideration.
Comment 1 Radu Stavila 2014-03-10 10:05:01 PDT
Created attachment 226313 [details]
Patch
Comment 2 Darin Adler 2014-03-10 10:12:31 PDT
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 3 Radu Stavila 2014-03-10 10:20:37 PDT
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 4 Antti Koivisto 2014-03-10 10:27:58 PDT
Comment on attachment 226313 [details]
Patch

r=me with the informative comment Darin asked for.
Comment 5 Radu Stavila 2014-03-10 10:33:10 PDT
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 6 WebKit Commit Bot 2014-03-10 11:42:58 PDT
Comment on attachment 226313 [details]
Patch

Clearing flags on attachment: 226313

Committed r165388: <http://trac.webkit.org/changeset/165388>
Comment 7 WebKit Commit Bot 2014-03-10 11:43:02 PDT
All reviewed patches have been landed.  Closing bug.