Bug 195374

Summary: Hit-testing of boxes over scrollers should account for border-radius
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: ScrollingAssignee: Antti Koivisto <koivisto>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Testcase
none
patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
patch
simon.fraser: review+
patch none

Description Simon Fraser (smfr) 2019-03-06 13:28:16 PST
Created attachment 363784 [details]
Testcase

Need to take border-radius into account for hit testing.
Comment 1 Radar WebKit Bug Importer 2019-03-06 13:28:41 PST
<rdar://problem/48649993>
Comment 2 Antti Koivisto 2019-03-28 07:02:03 PDT
Created attachment 366171 [details]
patch
Comment 3 EWS Watchlist 2019-03-28 07:04:34 PDT
Attachment 366171 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/Internals.h:350:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 EWS Watchlist 2019-03-28 13:52:44 PDT
Comment on attachment 366171 [details]
patch

Attachment 366171 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11693720

New failing tests:
fast/scrolling/ios/border-radious-event-region.html
Comment 5 EWS Watchlist 2019-03-28 13:52:45 PDT
Created attachment 366200 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 6 Antti Koivisto 2019-03-28 22:33:45 PDT
Created attachment 366251 [details]
patch
Comment 7 EWS Watchlist 2019-03-28 22:36:05 PDT
Attachment 366251 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/Internals.h:350:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Simon Fraser (smfr) 2019-03-29 11:08:14 PDT
Comment on attachment 366251 [details]
patch

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

> Source/WebCore/page/Frame.h:118
>      LayerTreeFlagsIncludeBackingStoreAttached = 1 << 7,
>      LayerTreeFlagsIncludeRootLayerProperties = 1 << 8,
> +    LayerTreeFlagsIncludeEventRegion = 1 << 9,

The cool kids are lining up the "= ..." these days :)

> Source/WebCore/platform/graphics/GraphicsLayer.cpp:935
> +        ts << indent << "(event region\n";
> +        for (auto& rect : m_eventRegion.rects()) {
> +            TextStream::IndentScope indentScope(ts);
> +            ts << indent << "(rect " << rect.x() << " " << rect.y() << " " << rect.width() << " " << rect.height() << ")\n";
> +        }
> +        ts << indent << ")\n";

Please add and use TextStream& operator<<(TextStream&, const Region&)

> Source/WebCore/platform/graphics/RoundedRect.h:117
> +    // Approximate by snipping away half-radii sized rectangles from corners.
> +    WEBCORE_EXPORT Region approximateAsRegion() const;

I think I'd prefer some kind of "stepping" function with a configurable step size at some point.

> LayoutTests/ChangeLog:10
> +        * fast/scrolling/ios/border-radious-event-region-expected.txt: Added.
> +        * fast/scrolling/ios/border-radious-event-region.html: Added.

Bad spelling
Comment 9 Antti Koivisto 2019-03-29 11:13:43 PDT
> > +    // Approximate by snipping away half-radii sized rectangles from corners.

That comment is a lie. We now snip away multiple rectangles based on arc length.
Comment 10 Antti Koivisto 2019-03-30 00:01:35 PDT
Created attachment 366360 [details]
patch
Comment 11 EWS Watchlist 2019-03-30 00:05:28 PDT
Attachment 366360 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/Internals.h:350:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 WebKit Commit Bot 2019-03-30 01:28:12 PDT
Comment on attachment 366360 [details]
patch

Clearing flags on attachment: 366360

Committed r243674: <https://trac.webkit.org/changeset/243674>
Comment 13 WebKit Commit Bot 2019-03-30 01:28:14 PDT
All reviewed patches have been landed.  Closing bug.