Bug 92612 - [Forms] Get rid of Element::isReadOnlyFormControl other than CSS related
Summary: [Forms] Get rid of Element::isReadOnlyFormControl other than CSS related
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks: 92602
  Show dependency treegraph
 
Reported: 2012-07-29 22:19 PDT by yosin
Modified: 2012-07-30 19:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch 1 (7.70 KB, patch)
2012-07-30 02:01 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (8.73 KB, patch)
2012-07-30 02:27 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 3 (8.68 KB, patch)
2012-07-30 03:07 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 4 (8.64 KB, patch)
2012-07-30 18:47 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.