Bug 195374 - Hit-testing of boxes over scrollers should account for border-radius
Summary: Hit-testing of boxes over scrollers should account for border-radius
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antti Koivisto
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-06 13:28 PST by Simon Fraser (smfr)
Modified: 2019-03-30 01:28 PDT (History)
4 users (show)

See Also:


Attachments
Testcase (1.17 KB, text/html)
2019-03-06 13:28 PST, Simon Fraser (smfr)
no flags Details
patch (21.40 KB, patch)
2019-03-28 07:02 PDT, Antti Koivisto
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (3.13 MB, application/zip)
2019-03-28 13:52 PDT, Build Bot
no flags Details
patch (21.42 KB, patch)
2019-03-28 22:33 PDT, Antti Koivisto
simon.fraser: review+
Details | Formatted Diff | Diff
patch (24.56 KB, patch)
2019-03-30 00:01 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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.