Bug 138775 – Use TextIndicator instead of the built in Lookup highlight

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Use TextIndicator instead of the built in Lookup highlight

[Tim Horton]

Created attachment 241680 [details]
Patch

[Tim Horton]

<rdar://problem/18953982>

[WebKit Commit Bot]

Attachment 241680 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/mac/TextIndicatorWindow.h:52:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/mac/TextIndicatorWindow.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:84:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:3070:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:3085:  Missing space inside { }.  [whitespace/braces] [5]
ERROR: Source/WebKit2/UIProcess/mac/TextIndicatorWindow.mm:123:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Anders Carlsson]

Comment on attachment 241680 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:3080
> +    _data->_textIndicatorWindow->setTextIndicator(textIndicator, fadeOut, animate, completionHandler);

Should WTF::move the completion handler.

> Source/WebKit2/UIProcess/mac/TextIndicatorWindow.h:52
> +    void setTextIndicator(PassRefPtr<TextIndicator>, bool fadeOut, bool animate, std::function<void ()> animationCompletionHandler);

Would be nice to combine fadeOut and animate into a set of flags.

[Tim Horton]

(In reply to comment #4)
> Comment on attachment 241680 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241680&action=review
> 
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:3080
> > +    _data->_textIndicatorWindow->setTextIndicator(textIndicator, fadeOut, animate, completionHandler);
> 
> Should WTF::move the completion handler.
> 
> > Source/WebKit2/UIProcess/mac/TextIndicatorWindow.h:52
> > +    void setTextIndicator(PassRefPtr<TextIndicator>, bool fadeOut, bool animate, std::function<void ()> animationCompletionHandler);
> 
> Would be nice to combine fadeOut and animate into a set of flags.

Agreed, especially because "fadeOut" is a less-than-ideal name for what it really does. Not in this patch, though.

[Tim Horton]

http://trac.webkit.org/changeset/176164

[Tim Horton]

Follow up to stop the crashes in http://trac.webkit.org/changeset/176166.