WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
130015
[CSS Regions] Hit-testing is not working properly inside scrollable regions
https://bugs.webkit.org/show_bug.cgi?id=130015
Summary
[CSS Regions] Hit-testing is not working properly inside scrollable regions
Radu Stavila
Reported
2014-03-10 04:04:41 PDT
Hit-testing on elements inside scrollable regions does not take the region's scroll offset into consideration.
Attachments
Patch
(9.31 KB, patch)
2014-03-10 10:05 PDT
,
Radu Stavila
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radu Stavila
Comment 1
2014-03-10 10:05:01 PDT
Created
attachment 226313
[details]
Patch
Darin Adler
Comment 2
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.
Radu Stavila
Comment 3
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());
Antti Koivisto
Comment 4
2014-03-10 10:27:58 PDT
Comment on
attachment 226313
[details]
Patch r=me with the informative comment Darin asked for.
Radu Stavila
Comment 5
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.
WebKit Commit Bot
Comment 6
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
>
WebKit Commit Bot
Comment 7
2014-03-10 11:43:02 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug