WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
92602
Change Element::isReadOnlyFormControl to Element::shouldMatchReadOnlySelector/shouldMatchReadWriteSelector or HTMLFormControlElement::readOnly
https://bugs.webkit.org/show_bug.cgi?id=92602
Summary
Change Element::isReadOnlyFormControl to Element::shouldMatchReadOnlySelector...
yosin
Reported
2012-07-29 19:16:36 PDT
This bug is preparation of
bug 92473
, :read-only and :read-write selector should match none-isFormisFormControlElement.
Attachments
Patch 1
(8.08 KB, patch)
2012-07-30 00:30 PDT
,
yosin
tkent
: review-
Details
Formatted Diff
Diff
Patch 2
(11.23 KB, patch)
2012-07-30 20:59 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch 3
(11.85 KB, patch)
2012-07-30 21:16 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch 4
(14.03 KB, patch)
2012-07-31 00:16 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
yosin
Comment 1
2012-07-29 23:41:30 PDT
***
Bug 92607
has been marked as a duplicate of this bug. ***
yosin
Comment 2
2012-07-30 00:30:49 PDT
Created
attachment 155224
[details]
Patch 1
yosin
Comment 3
2012-07-30 00:32:37 PDT
Comment on
attachment 155224
[details]
Patch 1 Could you review this patch? Thanks in advance.
Kent Tamura
Comment 4
2012-07-30 00:47:40 PDT
Comment on
attachment 155224
[details]
Patch 1 View in context:
https://bugs.webkit.org/attachment.cgi?id=155224&action=review
> Source/WebCore/dom/Element.h:354 > + virtual bool shouldMatchReadWriteSelector() const { return false; }
We should replace isReadOnlyFormControl() with shouldMatchReadWriteSelector in Element.h. If we have other isReadOnlyFormControl() callsites, we should remove them before this patch.
> Source/WebCore/html/HTMLInputElement.cpp:1479 > +bool HTMLInputElement::shouldMatchReadWriteSelector() const > +{ > + return m_inputType->shouldMatchReadWriteSelector(); > +}
We have no reasons to forward it to InputType::shouldMatchReadWriteSelector() for now. This change just wastes the code size and CPU time.
yosin
Comment 5
2012-07-30 19:16:39 PDT
Element::isReadOnlyFormControl() is used for :read-only and :read-write CSS pseudo class matching. We would like to rename to shouldMatchReadWriteSelector() to make intention of this function explicitly.
yosin
Comment 6
2012-07-30 20:59:01 PDT
Created
attachment 155431
[details]
Patch 2
yosin
Comment 7
2012-07-30 21:00:04 PDT
Comment on
attachment 155431
[details]
Patch 2 Could you review this patch? Thanks in advance.
Kent Tamura
Comment 8
2012-07-30 21:07:54 PDT
Comment on
attachment 155431
[details]
Patch 2 View in context:
https://bugs.webkit.org/attachment.cgi?id=155431&action=review
> Source/WebCore/dom/Element.h:353 > + virtual bool shouldMatchReadWriteSelector() const { return false; }
nit: we had better avoid to add the function body here for a virtual function. It wastes compile-time resources.
yosin
Comment 9
2012-07-30 21:16:59 PDT
Created
attachment 155432
[details]
Patch 3
yosin
Comment 10
2012-07-30 21:19:43 PDT
Comment on
attachment 155432
[details]
Patch 3 Clearing flags on attachment: 155432 Committed
r124171
: <
http://trac.webkit.org/changeset/124171
>
yosin
Comment 11
2012-07-30 21:19:50 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 12
2012-07-30 22:42:44 PDT
Re-opened since this is blocked by 92712
yosin
Comment 13
2012-07-31 00:16:41 PDT
Created
attachment 155452
[details]
Patch 4
yosin
Comment 14
2012-07-31 00:20:19 PDT
Comment on
attachment 155452
[details]
Patch 4 Could you review this patch? Thanks in advance. = Changes since the last patch = * Introduce Element::shouldMatchReadOnlySelector, because !shouldMatchReadWriteSelector isn't equal to read only control. ** We found this by failure of fast/css/square-button-appearance.html. This test uses "div" element for rendering square button.
Kent Tamura
Comment 15
2012-07-31 00:47:46 PDT
Comment on
attachment 155452
[details]
Patch 4 Looks good.
yosin
Comment 16
2012-07-31 00:59:01 PDT
Comment on
attachment 155452
[details]
Patch 4 Clearing flags on attachment: 155452 Committed
r124180
: <
http://trac.webkit.org/changeset/124180
>
yosin
Comment 17
2012-07-31 00:59:08 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.
Top of Page
Format For Printing
XML
Clone This Bug