Bug 92612

Summary: [Forms] Get rid of Element::isReadOnlyFormControl other than CSS related
Product: WebKit Reporter: yosin
Component: FormsAssignee: yosin
Status: RESOLVED FIXED    
Severity: Normal CC: eric, mifenton, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 92602    
Attachments:
Description Flags
Patch 1
none
Patch 2
none
Patch 3
none
Patch 4 none

Description yosin 2012-07-29 22:19:42 PDT 
Element::IsReadOnlyFormControl() was introduced for using :read-only/:read-write CSS pseudo classes. Although, it was used short cut for calling HTMLFormControlElement::readOnly().

Because of we have Element::shouldMatchReadWriteSelector() for this purpose, we don't need to have Element::IsReadOnlyFormControl() anymore.
Comment 1 yosin 2012-07-30 01:20:00 PDT 
We'll replace CSS related Element::isReadOnlyFormControl() to Element::shouldMatchReadWriteSelector()
Comment 2 yosin 2012-07-30 02:01:53 PDT 
Created attachment 155234 [details]
Patch 1
Comment 3 Build Bot 2012-07-30 02:10:48 PDT 
Comment on attachment 155234 [details]
Patch 1

Attachment 155234 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13396230
Comment 4 yosin 2012-07-30 02:27:14 PDT 
Created attachment 155237 [details]
Patch 2
Comment 5 yosin 2012-07-30 03:07:14 PDT 
Created attachment 155242 [details]
Patch 3
Comment 6 yosin 2012-07-30 03:08:05 PDT 
Comment on attachment 155242 [details]
Patch 3

Could you review this patch?
Thanks in advance.
Comment 7 Kent Tamura 2012-07-30 18:28:22 PDT 
Comment on attachment 155242 [details]
Patch 3

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

> Source/WebKit/blackberry/WebKitSupport/DOMSupport.cpp:243
> -    if (element->isReadOnlyFormControl() || !element->isEnabledFormControl())
> +    if (!element->isEnabledFormControl())
>          return false;
>  
>      if (isPopupInputField(element))
>          return false;
>  
> -    return element->isTextFormControl() || element->isContentEditable();
> +    if (element->isTextFormControl())
> +        return !static_cast<HTMLTextFormControl*>(element)->readOnly();
> +
> +    return element->isContentEditable();

Is it safe to change the evaluation order?
Comment 8 yosin 2012-07-30 18:47:34 PDT 
Created attachment 155417 [details]
Patch 4
Comment 9 yosin 2012-07-30 18:51:16 PDT 
Comment on attachment 155417 [details]
Patch 4

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* WebKit/blackberry/WebKitSupport/DOMSupport.cpp: Keep original evaluation order for read only control check.
Comment 10 Kent Tamura 2012-07-30 18:59:51 PDT 
Comment on attachment 155417 [details]
Patch 4

Looks good.
Comment 11 yosin 2012-07-30 19:01:37 PDT 
Comment on attachment 155417 [details]
Patch 4

Clearing flags on attachment: 155417

Committed r124146: <http://trac.webkit.org/changeset/124146>
Comment 12 yosin 2012-07-30 19:01:42 PDT 
All reviewed patches have been landed.  Closing bug.