WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
185282
Allow Web Touch events to timeout
https://bugs.webkit.org/show_bug.cgi?id=185282
Summary
Allow Web Touch events to timeout
Megan Gardner
Reported
2018-05-03 17:32:06 PDT
Allow Web Touch events to timeout
Attachments
Patch
(1.89 KB, patch)
2018-05-03 17:35 PDT
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Megan Gardner
Comment 1
2018-05-03 17:35:21 PDT
Created
attachment 339495
[details]
Patch
Megan Gardner
Comment 2
2018-05-03 17:36:23 PDT
<
rdar://problem/38728319
>
Tim Horton
Comment 4
2018-05-04 11:41:27 PDT
I think you should consider writing a test for this. Might need to add a mechanism to speed up timeouts (like the one we have to make them go to ∞).
Tim Horton
Comment 5
2018-05-04 11:42:57 PDT
ignoreSynchronousMessagingTimeoutsForTesting
WebKit Commit Bot
Comment 6
2018-05-04 12:16:46 PDT
Comment on
attachment 339495
[details]
Patch Clearing flags on attachment: 339495 Committed
r231370
: <
https://trac.webkit.org/changeset/231370
>
WebKit Commit Bot
Comment 7
2018-05-04 12:16:47 PDT
All reviewed patches have been landed. Closing bug.
Wenson Hsieh
Comment 8
2018-05-06 16:51:32 PDT
Comment on
attachment 339495
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=339495&action=review
> Source/WebKit/UIProcess/WebPageProxy.cpp:2303 > + handled = true;
I don't think this is quite right, since it would mean any touchstart that is dispatched to the page would behave as though the page called preventDefault(). This would prevent clicks when tapping on elements that have touch event handlers, but do not preventDefault() (e.g. links on Google search result pages).
Tim Horton
Comment 9
2018-05-06 18:48:17 PDT
Comment on
attachment 339495
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=339495&action=review
>> Source/WebKit/UIProcess/WebPageProxy.cpp:2303 >> + handled = true; > > I don't think this is quite right, since it would mean any touchstart that is dispatched to the page would behave as though the page called preventDefault(). This would prevent clicks when tapping on elements that have touch event handlers, but do not preventDefault() (e.g. links on Google search result pages).
Only if it times out, though? We thought that mostly made sense. I’ll add you to a thread about it.
Wenson Hsieh
Comment 10
2018-05-06 20:27:04 PDT
Comment on
attachment 339495
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=339495&action=review
>>> Source/WebKit/UIProcess/WebPageProxy.cpp:2303 >>> + handled = true; >> >> I don't think this is quite right, since it would mean any touchstart that is dispatched to the page would behave as though the page called preventDefault(). This would prevent clicks when tapping on elements that have touch event handlers, but do not preventDefault() (e.g. links on Google search result pages). > > Only if it times out, though? We thought that mostly made sense. I’ll add you to a thread about it.
Yeah, I think the plan makes sense (treat IPC time out as if default was prevented). Perhaps I'm missing something, but this change seems to make it so that as long as `replyReceived` is true (e.g. the IPC message didn't time out), then we set handled to true (i.e. treat it as if the page called preventDefault()).
Tim Horton
Comment 11
2018-05-06 20:37:04 PDT
Haha, I see, yes, it’s backwards :/
Tim Horton
Comment 12
2018-05-06 20:37:26 PDT
How in the world did that not cause a single test failure?!?
Wenson Hsieh
Comment 13
2018-05-06 20:39:55 PDT
(In reply to Tim Horton from
comment #12
)
> How in the world did that not cause a single test failure?!?
Touch-event-related tests cannot run on the open source bots :/ That said, our internal test runners are looking quite red...
Megan Gardner
Comment 14
2018-05-07 11:16:53 PDT
https://trac.webkit.org/changeset/231447/webkit
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