Summary: | Change Element::isReadOnlyFormControl to Element::shouldMatchReadOnlySelector/shouldMatchReadWriteSelector or HTMLFormControlElement::readOnly | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | yosin | ||||||||||
Component: | DOM | Assignee: | yosin | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cmarcelo, eric, macpherson, menard, mifenton, tkent, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 92607, 92612, 92712 | ||||||||||||
Bug Blocks: | 92473 | ||||||||||||
Attachments: |
|
Description
yosin
2012-07-29 19:16:36 PDT
*** Bug 92607 has been marked as a duplicate of this bug. *** Created attachment 155224 [details]
Patch 1
Comment on attachment 155224 [details]
Patch 1
Could you review this patch?
Thanks in advance.
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. 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. Created attachment 155431 [details]
Patch 2
Comment on attachment 155431 [details]
Patch 2
Could you review this patch?
Thanks in advance.
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. Created attachment 155432 [details]
Patch 3
Comment on attachment 155432 [details] Patch 3 Clearing flags on attachment: 155432 Committed r124171: <http://trac.webkit.org/changeset/124171> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by 92712 Created attachment 155452 [details]
Patch 4
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.
Comment on attachment 155452 [details]
Patch 4
Looks good.
Comment on attachment 155452 [details] Patch 4 Clearing flags on attachment: 155452 Committed r124180: <http://trac.webkit.org/changeset/124180> All reviewed patches have been landed. Closing bug. |