New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 422731 link

Starred by 13 users

Issue metadata

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



Sign in to add a comment

Implement Element.closest() API

Project Member Reported by pdr@chromium.org, Oct 12 2014

Issue description

This looks like a nice addition and would be a good merge candidate. It would be best to shoot a quick "intent to implement and ship" email to blink-dev before merging.

Webkit bug: https://bugs.webkit.org/show_bug.cgi?id=137418
Webkit patch: http://trac.webkit.org/changeset/174324
 

Comment 1 by tkent@chromium.org, Oct 15 2014

Labels: -Cr-Blink Cr-Blink-DOM

Comment 3 by rwlb...@gmail.com, Nov 6 2014

Same as  Issue 417603  ?
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 11 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=184946

------------------------------------------------------------------
r184946 | paritosh.in@samsung.com | 2014-11-07T02:57:38.565423Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.idl?r1=184946&r2=184945&pathrev=184946
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closeset.html?r1=184946&r2=184945&pathrev=184946
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=184946&r2=184945&pathrev=184946
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closeset-expected.txt?r1=184946&r2=184945&pathrev=184946
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.h?r1=184946&r2=184945&pathrev=184946

[WIP] Adding Element.closest() API

Implementing Element.closest() API according to specs
https://dom.spec.whatwg.org/#dom-element-closest

This API will parse selectors and if fails then throw SyntaxError
else return closest ancestor that matches selectors
if there is no ancestor that matches selectors, it will
return nullptr.

Blink-Dev Intent to Implement and ship Link :
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yk_Fo6hyvb8

WebKit Change: http://trac.webkit.org/changeset/174324

BUG= 422731 
R=habib.virji@samsung.com

Review URL: https://codereview.chromium.org/704733002
-----------------------------------------------------------------
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 11 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=184992

------------------------------------------------------------------
r184992 | pdr@chromium.org | 2014-11-07T22:29:26.841227Z

Changed paths:
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closeset.html?r1=184992&r2=184991&pathrev=184992
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=184992&r2=184991&pathrev=184992
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closeset-expected.txt?r1=184992&r2=184991&pathrev=184992
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.h?r1=184992&r2=184991&pathrev=184992
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.idl?r1=184992&r2=184991&pathrev=184992

Revert of Adding Element.closest() API (patchset #1 id:1 of https://codereview.chromium.org/704733002/)

Reason for revert:
Merged patch from WebKit contains errors.

Original issue's description:
> Adding Element.closest() API
> 
> Implementing Element.closest() API according to specs
> https://dom.spec.whatwg.org/#dom-element-closest
> 
> This API will parse selectors and if fails then throw SyntaxError
> else return closest ancestor that matches selectors
> if there is no ancestor that matches selectors, it will
> return nullptr.
> 
> Blink-Dev Intent to Implement and ship Link :
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yk_Fo6hyvb8
> 
> WebKit Change: http://trac.webkit.org/changeset/174324
> 
> BUG= 422731 
> R=habib.virji@samsung.com
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184946

TBR=habib.virji@samsung.com,rune@opera.com,l.gombos@samsung.com,paritosh.in@samsung.com
NOTREECHECKS=true
NOTRY=true
BUG= 422731 

Review URL: https://codereview.chromium.org/693203004
-----------------------------------------------------------------
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 12 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=185180

------------------------------------------------------------------
r185180 | paritosh.in@samsung.com | 2014-11-12T05:56:56.870755Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closest-general.html?r1=185180&r2=185179&pathrev=185180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=185180&r2=185179&pathrev=185180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/SelectorQuery.h?r1=185180&r2=185179&pathrev=185180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.h?r1=185180&r2=185179&pathrev=185180
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closest-scope-expected.txt?r1=185180&r2=185179&pathrev=185180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.idl?r1=185180&r2=185179&pathrev=185180
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closest-general-expected.txt?r1=185180&r2=185179&pathrev=185180
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/selectors/element-closest-scope.html?r1=185180&r2=185179&pathrev=185180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/SelectorQuery.cpp?r1=185180&r2=185179&pathrev=185180

Adding Element.closest() API 

Implementing Element.closest() API according to specs https://dom.spec.whatwg.org/#dom-element-closest

This API will parse selectors and if fails then throw SyntaxError
else return closest ancestor that matches selectors if there is
no ancestor that matches selectors, it will return nullptr.

Blink-Dev Intent to Implement and ship Link :
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yk_Fo6hyvb8

This patch is a merge from webkit http://trac.webkit.org/changeset/174324
by Dhi Aurrahman <diorahman@rockybars.com>".

BUG= 422731 ,  417603 
R=habib.virji@samsung.com

Review URL: https://codereview.chromium.org/701723007
-----------------------------------------------------------------

Comment 7 by rob@robwu.nl, Dec 7 2014

Philip, it seems that Element.closest did not make it in the Chrome 40.0.2214.x branch cut, did it?

This is the timeline of revisions:
Blink 184946 (landed; rolled in 3aa6240a4eea15156e1d213c8d6617144623f080, version 40.0.2213.0)
Blink 184992 (revert; rolled in 9e85caa5622b09c8c21545b7b8afd4f7ae992dc1, version 40.0.2214.0)
Blink 185180 (reland; rolled in c0e769b69fb60e6e71e8b46e7877b8f84f33dfe7, version 41.0.2218.0)

The Chrome 40 blog announcement asserts that Element.closest is supported:
http://blog.chromium.org/2014/12/chrome-40-beta-powerful-offline-and.html

Either the blog post should be rectified, or the patch should be merged with the 2214 branch.

(and shouldn't this bug be merged with  issue 417603 , by the way?)

Comment 8 by pdr@chromium.org, Dec 10 2014

Cc: jsb...@chromium.org rob@robwu.nl dominicc@google.com
@rob, thanks for the heads up! This feature isn't critical enough to merge this late in the game so lets punt until M41.

Josh or Dominic, could you please update the blogpost to remove the line about Element.closest?

Comment 9 by jsb...@chromium.org, Dec 11 2014

Heh, that was ghost-written. I didn't even know my name was on it until now.

Routed to the appropriate googlers to fix.
That bit on the blog post has been removed.

Thanks for keeping us on track, rob@ !

Comment 11 by rob@robwu.nl, Dec 11 2014

Labels: M-41
Status: Fixed
There are still many Tweets/G+/PR that refer to the blog post for Element.closest. I suggest to at least mention the feature, but struck through and with a comment that it will land in Chrome 41.
Cc: nagarjun...@samsung.com
 Issue 417603  has been merged into this issue.

Sign in to add a comment