New issue
Advanced search Search tips

Issue 682747 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Escaped asterisk or unicode token should not be a universal selector

Reported by r...@opera.com, Jan 19 2017

Issue description

The Blink CSS parser incorrectly recognizes an escaped '*' as a universal selector. Firefox doesn't.

<!DOCTYPE html>
<style>
\* { color: red }
\2a { background-color: pink }
</style>
<div>No red and no pink background</div>

 
Labels: Update-Quarterly

Comment 2 by shend@chromium.org, Oct 12 2017

Labels: Hotlist-GoodFirstBug
Labels: Code-Parser

Comment 4 by shend@chromium.org, Nov 3 2017

Owner: shend@chromium.org
Status: Started (was: Available)

Comment 5 by shend@chromium.org, Nov 3 2017

Labels: -Code-Parser
Actually, this doesn't look like a parser issue. The tokenizer correctly interprets "*" as a delimiter and "\*" as an ident. The problem is that CSSSelector represents universal selectors as a tag selector with name "*":

******* CSSSelector::Show("*") *******
  SelectorText(): *
  match_: 1
  GetPseudoType(): 0
  TagQName().LocalName(): *
  IsAttributeSelector(): 0
  Argument(): 
  Specificity(): 

Thus, "*" and "\*" form identical CSSSelectors. We could differentiate them either with a new selector match type or a boolean flag to say if it's a actually "*".

Comment 6 by shend@chromium.org, Nov 8 2017

It appears that adding a new selector type/boolean flag is actually quite difficult. The main problem is that we sometimes call into DOM code for matching, but the DOM code uses '*' as a wildcard as well. For the DOM code to differentiate between '*' and '\*', we have to either duplicate code or plumb this information down.

The cleanest solution I have is to use a sentinel string to represent '*'. The challenge is picking the actual sentinel value. Using something like Null will require some refactoring of the parsing code since Null is already being used as a sentinel.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69b7d1c14ebe8c5e82e825d6696126d106d0a110

commit 69b7d1c14ebe8c5e82e825d6696126d106d0a110
Author: Darren Shen <shend@chromium.org>
Date: Tue Dec 05 00:50:38 2017

Do not treat escaped asterisk as universal selector.

This patch fixes an issue where an escaped asterisk character is being
treated as a universal selector. The problem was that \* and * were
represented as g_star_atom (the '*' character) in selector matching.

To differentiate between the two, we picked a different way to represent
universal selectors. The local name of a universal selector becomes
g_null_atom instead of g_star_atom. This way, \* and * will no longer
clash.

Bug:  682747 
Change-Id: Ied14b468c8bfae6ed1022a565f4f46cea4c556a8
Reviewed-on: https://chromium-review.googlesource.com/765626
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521556}
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/LayoutTests/fast/css/css-selector-text-expected.txt
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/LayoutTests/fast/css/css-selector-text.html
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/CSSSelector.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/CSSSelector.h
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/PageRuleCollector.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/RuleFeatureSet.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/RuleSet.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/SelectorChecker.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/SelectorFilter.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/parser/CSSParserSelector.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.h
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/dom/NodeListsNodeData.h
[modify] https://crrev.com/69b7d1c14ebe8c5e82e825d6696126d106d0a110/third_party/WebKit/Source/core/dom/QualifiedName.cpp

Comment 8 by shend@chromium.org, Dec 5 2017

Status: Fixed (was: Started)

Sign in to add a comment