Bug 149304 - Heavy taps on links are sometimes interpreted as the preview gesture
Summary: Heavy taps on links are sometimes interpreted as the preview gesture
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-17 15:27 PDT by Beth Dakin
Modified: 2015-09-28 14:45 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.73 KB, patch)
2015-09-17 15:30 PDT, Beth Dakin
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2015-09-17 15:27:31 PDT
Heavy taps on links are sometimes interpreted as the preview gesture

rdar://problem/22689258
Comment 1 Beth Dakin 2015-09-17 15:30:11 PDT
Created attachment 261439 [details]
Patch
Comment 2 mitz 2015-09-17 16:06:12 PDT
Comment on attachment 261439 [details]
Patch

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

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:162
> +    double _lastPreviewStartTime;

Is there a way to use std::chrono types for this instead of a mix of doubles and floats?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3386
> +static const double gMaximumAccidentalPreviewTime = 250;

Probably no need for a named constant for this quantity if it’s only being used once. Definitely no need to use the “g” prefix.
Comment 3 Tim Horton 2015-09-17 16:21:40 PDT
Comment on attachment 261439 [details]
Patch

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

>> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h:162
>> +    double _lastPreviewStartTime;
> 
> Is there a way to use std::chrono types for this instead of a mix of doubles and floats?

Chrono would be good (at the very least, using double everywhere). You can store the time from std::chrono::steady_clock::now() and do something like "std::chrono::duration_cast<std::chrono::milliseconds>(
            std::chrono::steady_clock::now() - _lastPreviewStartTime)" and compare to the literal 250_ms

>> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3386
>> +static const double gMaximumAccidentalPreviewTime = 250;
> 
> Probably no need for a named constant for this quantity if it’s only being used once. Definitely no need to use the “g” prefix.

Agreed about the g. Don't care if you keep the named constant, but it should at least be inside the function, I guess?
Comment 4 Beth Dakin 2015-09-28 14:45:27 PDT
I rolled this patch out with http://trac.webkit.org/changeset/190284

We're going to try to fix this at a different level.