output should not match to :enabled or :disabled pseudo classes.

output should not match to :enabled or :disabled pseudo classes.

As per the spec
https://html.spec.whatwg.org/multipage/scripting.html#selector-enabled
https://html.spec.whatwg.org/multipage/scripting.html#concept-element-disabled

BUG=605518
Committed: https://crrev.com/25bd6595e89a77312b6de294025f3c6632aa0485
Cr-Commit-Position: refs/heads/master@{#389712}

[ramya.v, 2016-04-25 08:51:24]

ramya.v@samsung.com changed reviewers:
+ timloh@chromium.org, tkent@chromium.org

[ramya.v, 2016-04-25 08:51:24]

PTAL!

Thanks

[tkent, 2016-04-25 09:40:40]

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html
(right):

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html:2:
<script src="../../resources/js-test.js"></script>
I recommend to use testharness.js for conformance tests.

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html:3:
<style>
This STYLE element is unnecessary.

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:826: if
((element.isFormControlElement() && !isHTMLOutputElement(element)) ||
isHTMLOptionElement(element) || isHTMLOptGroupElement(element))
This is not a good change.  We should not add more element-specific logic to
SelectorChecker.

Please add Element::matchesEnabledPseudoClass(), and override it in
FormControlElement, HTMLOptionElement, HTMLOptGroupElement, and
HTMLOutputElement.

[ramya.v, 2016-04-25 14:47:29]

PTAL!
Thanks

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html
(right):

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html:2:
<script src="../../resources/js-test.js"></script>
On 2016/04/25 09:40:39, tkent wrote:
> I recommend to use testharness.js for conformance tests.

Done.

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/pseudo-enabled-disabled-output-element.html:3:
<style>
On 2016/04/25 09:40:40, tkent wrote:
> This STYLE element is unnecessary.

Done.

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/1914933003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:826: if
((element.isFormControlElement() && !isHTMLOutputElement(element)) ||
isHTMLOptionElement(element) || isHTMLOptGroupElement(element))
On 2016/04/25 09:40:40, tkent wrote:
> This is not a good change.  We should not add more element-specific logic to
> SelectorChecker.
> 
> Please add Element::matchesEnabledPseudoClass(), and override it in
> FormControlElement, HTMLOptionElement, HTMLOptGroupElement, and
> HTMLOutputElement.

Done.

[tkent, 2016-04-25 23:00:12]

https://codereview.chromium.org/1914933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/1914933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:826: if
(element.matchesEnabledPseudoClass())
I meant the code should be just |return element.matchesEnabledPseudoClass()|. 
I'm sorry that I didn't mention we should add matchesEnabledPseudoClass() to
HTMLAnchorElement and HTMLAreaElement too.

[ramya.v, 2016-04-26 05:27:00]

PTAL!

Thanks

https://codereview.chromium.org/1914933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/1914933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:826: if
(element.matchesEnabledPseudoClass())
On 2016/04/25 23:00:12, tkent wrote:
> I meant the code should be just |return element.matchesEnabledPseudoClass()|. 
> I'm sorry that I didn't mention we should add matchesEnabledPseudoClass() to
> HTMLAnchorElement and HTMLAreaElement too.

Done.
Since HTMLAreaElement inherits from HTMLAnchorElement, added
matchesEnabledPseudoClass only in HTMLAnchorElement in protected section.

[tkent, 2016-04-26 05:33:51]

lgtm.

Thank you for the cleaner code!

[tkent, 2016-04-26 05:33:55]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-04-26 05:34:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1914933003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1914933003/40001

[commit-bot: I haz the power, 2016-04-26 06:57:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-26 06:58:30]

Description was changed from

==========
output should not match to :enabled or :disabled pseudo classes.

As per the spec
https://html.spec.whatwg.org/multipage/scripting.html#selector-enabled
https://html.spec.whatwg.org/multipage/scripting.html#concept-element-disabled

BUG=605518
==========

to

==========
output should not match to :enabled or :disabled pseudo classes.

As per the spec
https://html.spec.whatwg.org/multipage/scripting.html#selector-enabled
https://html.spec.whatwg.org/multipage/scripting.html#concept-element-disabled

BUG=605518

Committed: https://crrev.com/25bd6595e89a77312b6de294025f3c6632aa0485
Cr-Commit-Position: refs/heads/master@{#389712}
==========

[commit-bot: I haz the power, 2016-04-26 06:58:32]

Patchset 3 (id:??) landed as
https://crrev.com/25bd6595e89a77312b6de294025f3c6632aa0485
Cr-Commit-Position: refs/heads/master@{#389712}