Bug 92612

Summary:

[Forms] Get rid of Element::isReadOnlyFormControl other than CSS related

Product:

WebKit

Reporter:

yosin

Component:

Forms

Assignee:

yosin

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, mifenton, tkent, webkit.review.bot

Priority:

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

yosin

Reported 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.

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

yosin

Comment 1 2012-07-30 01:20:00 PDT

We'll replace CSS related Element::isReadOnlyFormControl() to Element::shouldMatchReadWriteSelector()

yosin

Comment 2 2012-07-30 02:01:53 PDT

Created attachment 155234 [details] Patch 1

Build Bot

Comment 3 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

yosin

Comment 4 2012-07-30 02:27:14 PDT

Created attachment 155237 [details] Patch 2

yosin

Comment 5 2012-07-30 03:07:14 PDT

Created attachment 155242 [details] Patch 3

yosin

Comment 6 2012-07-30 03:08:05 PDT

Comment on attachment 155242 [details] Patch 3 Could you review this patch? Thanks in advance.

Kent Tamura

Comment 7 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?

yosin

Comment 8 2012-07-30 18:47:34 PDT

Created attachment 155417 [details] Patch 4

yosin

Comment 9 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.

Kent Tamura

Comment 10 2012-07-30 18:59:51 PDT

Comment on attachment 155417 [details] Patch 4 Looks good.

yosin

Comment 11 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>

yosin

Comment 12 2012-07-30 19:01:42 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.