Bug 72930 – window.getMatchedCSSRules() not supporting pseudo element

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Naveen Bobbili]

What steps will reproduce the problem?
1. Create CSS rules that apply to an element (including pseudo-element selectors)
2. Call window.getMatchedCSSRules(domElement, pseudoElementString), with pseudoElementString corresponding to those used in step 1.

What is the expected result?
The method should return a rule list containing the rules with pseudo element/class selectors.

What happens instead?
It only returns a rule list with the non-pseudo element/class selectors.

Please provide any additional information below. Attach a screenshot if
possible.
I've provided a html page which is a demo of the bug. Additionally, it would be very convenient to be able to call this method and have it return all rules applying to the passed element for all pseudo elements/classes in one call. Further, it would also be convenient to optionally return the rule list in order of selector specificity, or provide the specificity value as a part of the CSSRule object.

[Naveen Bobbili]

Created attachment 116310 [details]
Test case to reproduce the issue.

[Naveen Bobbili]

This is a chromium Bug
http://code.google.com/p/chromium/issues/detail?id=102599

[Naveen Bobbili]

Created attachment 116344 [details]
Proposed Patch

Modified DOMWindow::getMatchedCSSRules() for getting the CSS Rules of pseudo elements.

[WebKit Review Bot]

Attachment 116344 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

LayoutTests/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Naveen Bobbili]

Created attachment 116345 [details]
Updated Patch

Fixed Style.

[Naveen Bobbili]

Created attachment 116701 [details]
Updated Patch

As per review comments.

[Ryosuke Niwa]

Comment on attachment 116701 [details]
Updated Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116701&action=review

> Source/WebCore/ChangeLog:9
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the description ("Added functionality to retrieve CSS rules of psuedo elements using getMatchedCSSRules.") followed by a blank line. See other entries.

> Source/WebCore/page/DOMWindow.cpp:1355
> +PassRefPtr<CSSRuleList> DOMWindow::getMatchedCSSRules(Element* element, const String& pseudoElt, bool authorOnly) const

Please don't use abbreviations like Elt. Spell out Element.

[Naveen Bobbili]

Created attachment 116705 [details]
Updated Patch

As per review comments.

[Simon Fraser (smfr)]

Comment on attachment 116705 [details]
Updated Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116705&action=review

> Source/WebCore/page/DOMWindow.cpp:1370
> +    return m_frame->document()->styleSelector()->pseudoStyleRulesForElement(element, pseudoId, rulesToInclude);

Is there any benefit to the old code path when pseudoElement is empty?

> LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements.html:22
> +var cssText = "border-top-width: 1pxborder-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-style: initial; border-color: initial; border-image: initial;"

Is there a missing semicolon at "1pxborder"?

[Naveen Bobbili]

(In reply to comment #9)
> (From update of attachment 116705 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=116705&action=review
> 
> > Source/WebCore/page/DOMWindow.cpp:1370
> > +    return m_frame->document()->styleSelector()->pseudoStyleRulesForElement(element, pseudoId, rulesToInclude);
> 
> Is there any benefit to the old code path when pseudoElement is empty?
> 
No there is no change. The empty pseudoElement still returns the same value.

> > LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements.html:22
> > +var cssText = "border-top-width: 1pxborder-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-style: initial; border-color: initial; border-image: initial;"
> 
> Is there a missing semicolon at "1pxborder"?
Yes there is. I will fix it and upload the patch.

[Naveen Bobbili]

Created attachment 116881 [details]
Updated Patch

Fixed Review comments.

[Simon Fraser (smfr)]

Comment on attachment 116881 [details]
Updated Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116881&action=review

> LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements.html:19
> +var styled = document.getElementById('div1');

'styled' doesn't reflect what this variable represents.

> LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements.html:22
> +var cssText = "border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-style: initial; border-color: initial; border-image: initial;"

This doesn't appear to be used.

[Naveen Bobbili]

Created attachment 117114 [details]
Updated Patch

Fixed Review comments.

[Darin Adler]

Comment on attachment 117114 [details]
Updated Patch

Would be better if the test case had more coverage.

[WebKit Review Bot]

Comment on attachment 117114 [details]
Updated Patch

Clearing flags on attachment: 117114

Committed r101587: <http://trac.webkit.org/changeset/101587>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.