Fix selector namespace prefix resolution.

Fix selector namespace prefix resolution.

When parsed without a stylesheet context, there are no prefix to
namespace URI mapping, so a ns name prefixed selector should be
invalid. Instead, we mapped "ns|e" to "*|e".

Here, we instead make "ns|e" invalid in contexts where there is no
stylesheet. However, "|e" and "*|e" should still be valid, so that part
of the prefix resolution is moved from the stylesheet to the selector
parser.

This meant that prefix resolution was incorrect for the select
attribute of the content element.

Also, prefixes in Selectors API were handled outside of the selector
parsing instead. Now we handle it inside the selector parsing instead
which means that we throw a SyntaxError instead of a NamespaceError for
unresolved namespace prefixes in the Selectors API. This is in line
with the specifications[1][2] and Gecko.

Another issue, was that setting selectorText of StyleRule did not pass
a stylesheet to the selector parser, so namespace resolution did not
work for setting selectorText.

[1] https://www.w3.org/TR/selectors-api2/#resolving-namespaces
[2] https://dom.spec.whatwg.org/#scope-match-a-selectors-string

R=timloh@chromium.org
BUG=580023, 580445
Committed: https://crrev.com/8ccb69f37572249545fc7b1b8bab97fe71910321
Cr-Commit-Position: refs/heads/master@{#371275}

[rune, 2016-01-22 11:47:29]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 11:47:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1616423003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1616423003/1

[rune, 2016-01-22 11:50:30]

ptal

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

https://codereview.chromium.org/1616423003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:308:
m_failedParsing = true;
There's a separate CL (https://codereview.chromium.org/1625433002/) for this,
but I needed the fix to not have regressions in the exception throwing for
querySelector and friends.

[commit-bot: I haz the power, 2016-01-22 13:15:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-22 13:15:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rune, 2016-01-22 13:40:39]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 13:40:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1616423003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1616423003/20001

[rune, 2016-01-22 13:45:46]

The CQ bit was unchecked by rune@opera.com

[rune, 2016-01-22 13:48:23]

The CQ bit was checked by rune@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 13:48:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1616423003/20002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1616423003/20002

[commit-bot: I haz the power, 2016-01-22 14:59:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-22 14:59:47]

Dry run: This issue passed the CQ dry run.

[Timothy Loh, 2016-01-25 00:30:47]

lgtm!

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSPageRule.cpp (right):

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSPageRule.cpp:73: CSSSelectorList
selectorList = CSSParser::parseSelector(context, parentStyleSheet()->contents(),
selectorText);
OK, but I wonder if this function is completely wrong,
CSSParserImpl::consumePageRule just does it's own thing.

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleRule.cpp (right):

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleRule.cpp:98: CSSSelectorList
selectorList = CSSParser::parseSelector(context, parentStyleSheet()->contents(),
selectorText);
You probably need to null-check parentStyleSheet (here and CSSPageRule), it ends
up null in some cases (fast/dom/css-delete-doc.html).

[rune, 2016-01-25 08:58:48]

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSPageRule.cpp (right):

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSPageRule.cpp:73: CSSSelectorList
selectorList = CSSParser::parseSelector(context, parentStyleSheet()->contents(),
selectorText);
On 2016/01/25 00:30:47, Timothy Loh wrote:
> OK, but I wonder if this function is completely wrong,
> CSSParserImpl::consumePageRule just does it's own thing.

Reported https://crbug.com/580956

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleRule.cpp (right):

https://codereview.chromium.org/1616423003/diff/20002/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleRule.cpp:98: CSSSelectorList
selectorList = CSSParser::parseSelector(context, parentStyleSheet()->contents(),
selectorText);
On 2016/01/25 00:30:47, Timothy Loh wrote:
> You probably need to null-check parentStyleSheet (here and CSSPageRule), it
ends
> up null in some cases (fast/dom/css-delete-doc.html).

Done.

[rune, 2016-01-25 08:59:09]

The CQ bit was checked by rune@opera.com

[rune, 2016-01-25 08:59:10]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1616423003/#ps70001
(title: "Fixed review issues")

[commit-bot: I haz the power, 2016-01-25 08:59:36]

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

[commit-bot: I haz the power, 2016-01-25 09:08:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-25 09:08:05]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[rune, 2016-01-25 09:22:53]

rune@opera.com changed reviewers:
+ dgozman@chromium.org

[rune, 2016-01-25 09:22:53]

@dgozman: need review for Source/web

[dgozman, 2016-01-25 18:05:33]

On 2016/01/25 09:22:53, rune wrote:
> @dgozman: need review for Source/web

Source/web lgtm

[rune, 2016-01-25 18:32:22]

The CQ bit was checked by rune@opera.com

[commit-bot: I haz the power, 2016-01-25 18:33:27]

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

[commit-bot: I haz the power, 2016-01-25 19:19:35]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2016-01-25 19:21:16]

Description was changed from

==========
Fix selector namespace prefix resolution.

When parsed without a stylesheet context, there are no prefix to
namespace URI mapping, so a ns name prefixed selector should be
invalid. Instead, we mapped "ns|e" to "*|e".

Here, we instead make "ns|e" invalid in contexts where there is no
stylesheet. However, "|e" and "*|e" should still be valid, so that part
of the prefix resolution is moved from the stylesheet to the selector
parser.

This meant that prefix resolution was incorrect for the select
attribute of the content element.

Also, prefixes in Selectors API were handled outside of the selector
parsing instead. Now we handle it inside the selector parsing instead
which means that we throw a SyntaxError instead of a NamespaceError for
unresolved namespace prefixes in the Selectors API. This is in line
with the specifications[1][2] and Gecko.

Another issue, was that setting selectorText of StyleRule did not pass
a stylesheet to the selector parser, so namespace resolution did not
work for setting selectorText.

[1] https://www.w3.org/TR/selectors-api2/#resolving-namespaces
[2] https://dom.spec.whatwg.org/#scope-match-a-selectors-string

R=timloh@chromium.org
BUG=580023,580445
==========

to

==========
Fix selector namespace prefix resolution.

When parsed without a stylesheet context, there are no prefix to
namespace URI mapping, so a ns name prefixed selector should be
invalid. Instead, we mapped "ns|e" to "*|e".

Here, we instead make "ns|e" invalid in contexts where there is no
stylesheet. However, "|e" and "*|e" should still be valid, so that part
of the prefix resolution is moved from the stylesheet to the selector
parser.

This meant that prefix resolution was incorrect for the select
attribute of the content element.

Also, prefixes in Selectors API were handled outside of the selector
parsing instead. Now we handle it inside the selector parsing instead
which means that we throw a SyntaxError instead of a NamespaceError for
unresolved namespace prefixes in the Selectors API. This is in line
with the specifications[1][2] and Gecko.

Another issue, was that setting selectorText of StyleRule did not pass
a stylesheet to the selector parser, so namespace resolution did not
work for setting selectorText.

[1] https://www.w3.org/TR/selectors-api2/#resolving-namespaces
[2] https://dom.spec.whatwg.org/#scope-match-a-selectors-string

R=timloh@chromium.org
BUG=580023,580445

Committed: https://crrev.com/8ccb69f37572249545fc7b1b8bab97fe71910321
Cr-Commit-Position: refs/heads/master@{#371275}
==========

[commit-bot: I haz the power, 2016-01-25 19:21:17]

Patchset 5 (id:??) landed as
https://crrev.com/8ccb69f37572249545fc7b1b8bab97fe71910321
Cr-Commit-Position: refs/heads/master@{#371275}