Bug 145780

Summary: Simplify event handling logic for search cancel button
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, cfleizach, darin, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 145774    
Bug Blocks:    
Attachments:
Description Flags
Patch darin: review+

Description Daniel Bates 2015-06-08 18:18:01 PDT 
Following the patch for bug #145774, we can take advantage of the click event to simplify the event handling logic for the search cancel button.
Comment 1 Daniel Bates 2015-06-08 18:26:29 PDT 
Created attachment 254534 [details]
Patch
Comment 2 Darin Adler 2015-06-09 11:31:41 PDT 
Comment on attachment 254534 [details]
Patch

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

r=me if you figure out how to make this work or compile it out on iOS

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:176
> +    setAttribute(aria_labelAttr, AtomicString(AXSearchFieldCancelButtonText()));

iOS build failure:

html/shadow/TextControlInnerElements.cpp:176:47: error: use of undeclared identifier 'AXSearchFieldCancelButtonText'
    setAttribute(aria_labelAttr, AtomicString(AXSearchFieldCancelButtonText()));

That is in a !PLATFORM(IOS) block.

Separate question: Is the explicit AtomicString construction required here?

> Source/WebCore/html/shadow/TextControlInnerElements.cpp:201
> +        input->setValueForUser("");

Please use emptyString() rather than "".
Comment 3 Daniel Bates 2015-06-11 10:16:45 PDT 
(In reply to comment #2)
> > Source/WebCore/html/shadow/TextControlInnerElements.cpp:176
> > +    setAttribute(aria_labelAttr, AtomicString(AXSearchFieldCancelButtonText()));
> 
> iOS build failure:
> 
> html/shadow/TextControlInnerElements.cpp:176:47: error: use of undeclared
> identifier 'AXSearchFieldCancelButtonText'
>     setAttribute(aria_labelAttr,
> AtomicString(AXSearchFieldCancelButtonText()));
> 
> That is in a !PLATFORM(IOS) block.

Will add !PLATFORM(IOS)-guard around this code. 

> 
> Separate question: Is the explicit AtomicString construction required here?

No, it is not required. Will remove.
 
> 
> > Source/WebCore/html/shadow/TextControlInnerElements.cpp:201
> > +        input->setValueForUser("");
> 
> Please use emptyString() rather than "".

Will substitute emptyString() for "".
Comment 4 Daniel Bates 2015-06-11 10:21:28 PDT 
Committed r185459: <http://trac.webkit.org/changeset/185459>