Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(325)

Issue 26218002: ClassRootNodeList should start from rootNode, not the first child of rootNode. (Closed)

Created:
7 years, 2 months ago by tasak
Modified:
7 years, 2 months ago
Reviewers:
haraken
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, adamk+blink_chromium.org
Visibility:
Public.

Description

ClassRootNodeList should start from rootNode, not the first child of rootNode. When some selector like ".class > div" (a class selector is not the right-most one), ClassRootNodeList is used to find all elements which match the class selector (in this case, .class). Suppose that we have the following DOM tree and we try document.body.querySelector('.class > div') and document.body.querySelectorAll('class > div'). <body class='.class'> <div></div> </body> If ClassRootNodeList starts from a given rootNode, i.e. document.body, since there are no child elements of body whose class attirbute is "class", the querySelector and querySelectorAll doesn't match any elements. This is a problem. BUG=304069 TEST=fast/selectors/querySelector-leftmost-selector-matches-rootNode.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159014

Patch Set 1 #

Total comments: 2

Patch Set 2 : Patch for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -3 lines) Patch
A LayoutTests/fast/selectors/querySelector-leftmost-selector-matches-rootNode.html View 1 chunk +28 lines, -0 lines 0 comments Download
A LayoutTests/fast/selectors/querySelector-leftmost-selector-matches-rootNode-expected.txt View 1 chunk +18 lines, -0 lines 0 comments Download
M Source/core/dom/SelectorQuery.cpp View 1 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
tasak
7 years, 2 months ago (2013-10-07 05:55:04 UTC) #1
haraken
LGTM
7 years, 2 months ago (2013-10-07 07:08:18 UTC) #2
haraken
https://codereview.chromium.org/26218002/diff/1/Source/core/dom/SelectorQuery.cpp File Source/core/dom/SelectorQuery.cpp (right): https://codereview.chromium.org/26218002/diff/1/Source/core/dom/SelectorQuery.cpp#newcode67 Source/core/dom/SelectorQuery.cpp:67: explicit ClassRootNodeList(Node* rootNode, const AtomicString& className) Nit: 'explicit' is ...
7 years, 2 months ago (2013-10-07 07:08:49 UTC) #3
tasak
Thank you for reviewing. https://codereview.chromium.org/26218002/diff/1/Source/core/dom/SelectorQuery.cpp File Source/core/dom/SelectorQuery.cpp (right): https://codereview.chromium.org/26218002/diff/1/Source/core/dom/SelectorQuery.cpp#newcode67 Source/core/dom/SelectorQuery.cpp:67: explicit ClassRootNodeList(Node* rootNode, const AtomicString& ...
7 years, 2 months ago (2013-10-07 07:34:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tasak@google.com/26218002/8001
7 years, 2 months ago (2013-10-07 07:34:52 UTC) #5
commit-bot: I haz the power
7 years, 2 months ago (2013-10-07 09:11:46 UTC) #6
Message was sent while issue was closed.
Change committed as 159014

Powered by Google App Engine
This is Rietveld 408576698