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-
Patch 2 (11.23 KB, patch)
2012-07-30 20:59 PDT, yosin
no flags
Patch 3 (11.85 KB, patch)
2012-07-30 21:16 PDT, yosin
no flags
Patch 4 (14.03 KB, patch)
2012-07-31 00:16 PDT, yosin
no flags
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
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
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
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
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.