WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
193687
Long presses on YouTube links incorrectly trigger a load
https://bugs.webkit.org/show_bug.cgi?id=193687
Summary
Long presses on YouTube links incorrectly trigger a load
Megan Gardner
Reported
2019-01-22 14:44:44 PST
Cancel Web Touches Properly
Attachments
Patch
(2.41 KB, patch)
2019-01-22 14:50 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch
(2.51 KB, patch)
2019-01-22 15:13 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch
(2.56 KB, patch)
2019-01-22 15:48 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch for landing
(2.57 KB, patch)
2019-01-22 17:57 PST
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Megan Gardner
Comment 1
2019-01-22 14:50:25 PST
Created
attachment 359780
[details]
Patch
Tim Horton
Comment 2
2019-01-22 15:08:40 PST
Comment on
attachment 359780
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359780&action=review
> Source/WebKit/ChangeLog:3 > + Cancel Web Touches Properly
Can(cel) This Bug & Patch Have A Title That Describes The Symptom?
> Source/WebKit/ChangeLog:5 > +
And Probably A Radar #?
Megan Gardner
Comment 3
2019-01-22 15:13:01 PST
Created
attachment 359786
[details]
Patch
Tim Horton
Comment 4
2019-01-22 15:16:28 PST
Comment on
attachment 359786
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359786&action=review
> Source/WebKit/Platform/spi/ios/UIKitSPI.h:761 > +- (void)cancel;
Was this here forever into the past, or do you need some ifdefs?
> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1354 > +- (void)_cancelWebGestureRecognizer > +{ > + [_touchEventGestureRecognizer cancel]; > +}
Why the indirection? Why "Web" gesture recognizer? Aren't they all technically about the web? :D I could see _cancelTouchEventsGestureRecognizer or something, but also think you just don't need this function.
Megan Gardner
Comment 5
2019-01-22 15:48:27 PST
Created
attachment 359791
[details]
Patch
Tim Horton
Comment 6
2019-01-22 16:10:46 PST
Comment on
attachment 359791
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359791&action=review
> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 > +- (void)_cancelWebGestureRecognizer
Still not sure why the method. Are you thinking it will have more callers? And my other complaint about the method name stills stands.
> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1353 > +#if HAVE(CANCEL_WEB_TOUCH_EVENTS_GESTURE)
I think the name I provided before was closer to the other names in e.g. Platform.h, but I guess this is OK.
Megan Gardner
Comment 7
2019-01-22 16:30:55 PST
Comment on
attachment 359791
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359791&action=review
>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 >> +- (void)_cancelWebGestureRecognizer > > Still not sure why the method. Are you thinking it will have more callers? > And my other complaint about the method name stills stands.
They are this is the Web Gesture in all other instances, because it is the gesture that passes the touches to the web. I think this helps clear up that is this the gesture we are talking about. And while there are no additional callers today, there might be in the future, so I'd like to encapsulate the need for feature defines. Also, several other gestures use this exact layout of code to do the exact same thing.
Tim Horton
Comment 8
2019-01-22 16:38:51 PST
Comment on
attachment 359791
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=359791&action=review
>>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 >>> +- (void)_cancelWebGestureRecognizer >> >> Still not sure why the method. Are you thinking it will have more callers? >> And my other complaint about the method name stills stands. > > They are this is the Web Gesture in all other instances, because it is the gesture that passes the touches to the web. I think this helps clear up that is this the gesture we are talking about. > And while there are no additional callers today, there might be in the future, so I'd like to encapsulate the need for feature defines. Also, several other gestures use this exact layout of code to do the exact same thing.
I don't think it's ever called just "Web", that would be crazy. It is "WebTouchEvent" in UIKit, and "TouchEvent" in WebKit. You can decide what to call it.
Megan Gardner
Comment 9
2019-01-22 17:57:05 PST
Created
attachment 359825
[details]
Patch for landing
Megan Gardner
Comment 10
2019-01-22 17:59:12 PST
<
rdar://problem/47056717
>
WebKit Commit Bot
Comment 11
2019-01-22 18:12:14 PST
Comment on
attachment 359825
[details]
Patch for landing Clearing flags on attachment: 359825 Committed
r240316
: <
https://trac.webkit.org/changeset/240316
>
WebKit Commit Bot
Comment 12
2019-01-22 18:12:15 PST
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