Bug 135918 - [Services with UI] Use a longer delay duration for editable content
Summary: [Services with UI] Use a longer delay duration for editable content
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-08-13 17:20 PDT by Enrica Casucci
Modified: 2014-08-13 17:40 PDT (History)
0 users

See Also:

Attachments
Patch (1.70 KB, patch)
2014-08-13 17:22 PDT, Enrica Casucci 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2014-08-13 17:20:17 PDT 
Use a longer delay before showing the service UI.

<rdar://problem/17998929>
Comment 1 Enrica Casucci 2014-08-13 17:22:46 PDT 
Created attachment 236568 [details]
Patch
Comment 2 Tim Horton 2014-08-13 17:25:57 PDT 
Comment on attachment 236568 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:408
> +    std::chrono::steady_clock::duration minimumTimeUntilHighlightShouldBeShown = m_webPage.corePage()->focusController().focusedOrMainFrame().selection().selection().isContentEditable() ? 1000_ms : 200_ms;

This line is crazy long; could we maybe pull the 'isContentEditable' check out onto its own line? Also, could we maybe move the two constants to the top of the file?
Comment 3 Enrica Casucci 2014-08-13 17:33:29 PDT 
(In reply to comment #2)
> (From update of attachment 236568 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236568&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:408
> > +    std::chrono::steady_clock::duration minimumTimeUntilHighlightShouldBeShown = m_webPage.corePage()->focusController().focusedOrMainFrame().selection().selection().isContentEditable() ? 1000_ms : 200_ms;
> 
> This line is crazy long; could we maybe pull the 'isContentEditable' check out onto its own line? Also, could we maybe move the two constants to the top of the file?
I can split the line. I prefer not to move the constants since they are only used in this method.
Comment 4 Enrica Casucci 2014-08-13 17:40:09 PDT 
Committed revision 172546.