701419 - Inspector should always provide a unique selector

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Dietrich Ayala (:dietrich)]

Currently when using the inspector, it sometimes shows "td". This is not helpful if I'm using Inspector to do actual things with the inspected element.

We should provide access to a truly unique selector for the inspected element.

It doesn't make much sense to do it in the breadcrumb, but you could definitely do it in the selector popup that displays on hover/click of an element.

Comment 1

[Rob Campbell [:rc] (:robcee)]

hey Dietrich,

I guess you mean that you're inspecting a <td> element in a table and you have no way to refer to it.

You can grab the currently-selected node in the web console via the $0 convenience variable, but maybe you want something else here. Some way to "copy a reference to this element".

We could provide a menu option (maybe on the breadcrumbs or the selected node itself) to copy the XPath for the node. Or generate a unique selector for it, which might be tricky.

... some quick googling ...

This could work:

http://stackoverflow.com/questions/3620116/get-css-path-from-dom-element

Comment 2

[Dietrich Ayala (:dietrich)]

Yes, I want a string CSS selector for the current node that uniquely identifies it.

This would close a workflow hole for actually doing things like writing CSS or JS code while using the Inspector.

Comment 3

[Joe Walker [:jwalker]]

Here's the algo that I'm thinking of:

1. Does the element have an id? If so, check that getElementById returns that element (there could be duplicate ids) and use #id

2. Does the element have a class? If so querySelectorAll, and loop searching for the element, and use .class:nth-child(i)

3. Does the element have a parent? if not, loop over the children of the root element and use tagname:nth-child(i)

4. Do steps 1, 2 and 3 on the parent element and use 'parent > tagname:nth-child(i)'

There are lots of tweaks we could do to this, like looking for parent=body rather than parent=null in step 3. However I suspect this will come up with a fairly short unique CSS expression in all but pathalogical cases.

Optimizations welcome.

Comment 4

[Brian Burg]

FYI

Uniquely identifying a DOM node is a common dilemma for my research area, specifically capture/replay of DOM events. In order to reproduce event dispatch, one must of course re-locate the event target.

Pure JavaScript deterministic replay systems (f.e., Mugshot http://research.microsoft.com/apps/pubs/default.aspx?id=120937) use the algorithm as described above, though in terms of DOM traversal actions, not selectors. This is easy to understand but requires space roughly proportional to tree depth. My research project doesn't need to surface a unique selector to the user, so I instead identify a node by its order in a well-defined DOM node traversal. The code that does that in WK is below

http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.cpp#L2613

Comment 5

[Joe Walker [:jwalker]]

For what it's worth I have an implementation of this:

https://github.com/joewalker/gcli/blob/6e47ca629f159e0c98ba7fdbf6eb0f30ed850145/lib/gcli/util.js#L213

Seems to work OK. Has tests. Anyone desperate for this?

Comment 6

[Joe Walker [:jwalker]]

(In reply to Joe Walker from comment #5)
> For what it's worth I have an implementation of this:
> 
> https://github.com/joewalker/gcli/blob/
> 6e47ca629f159e0c98ba7fdbf6eb0f30ed850145/lib/gcli/util.js#L213
> 
> Seems to work OK. Has tests. Anyone desperate for this?

I'm taking that silence as a 'no'. We can integrate when we are.

Comment 7

[Rob Campbell [:rc] (:robcee)]

desperate isn't quite accurate but I'd like to have it in one of our shared libs.

I'm working on a context menu for the HTML panel (bug 584407) that would benefit from this greatly.

Comment 9

[Christos Stathis]

Attachment: Adds a "Copy Unique Selector" option in the inspected element menu

Hello all,

I gave it a try. The patch adds a "Copy Unique Selector" option in the inspected element 's menu. Any feedback is welcome.

Comment 10

[Paul Rouget [:paul]]

Comment on attachment 708068 [details] [diff] [review]
Adds a "Copy Unique Selector" option in the inspected element menu

(In reply to Christos Stathis from comment #9)
> I gave it a try. The patch adds a "Copy Unique Selector" option in the
> inspected element 's menu. Any feedback is welcome.

Wow, thanks! I've been waiting for that for a while :)
It appears to work well. Code is good.
Can you add a test? (see browser/devtools/inspector/test/)
If you need any assistance for this, please ping me on IRC (#devtools).

>diff -r 126f887730f5 browser/devtools/inspector/InspectorPanel.jsm
>--- a/browser/devtools/inspector/InspectorPanel.jsm	Tue Jan 29 11:40:50 2013 -0500
>+++ b/browser/devtools/inspector/InspectorPanel.jsm	Tue Jan 29 23:17:00 2013 +0200
>@@ -7,16 +7,19 @@
> const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
> 
> this.EXPORTED_SYMBOLS = ["InspectorPanel"];
> 
> Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> Cu.import("resource://gre/modules/Services.jsm");
> Cu.import("resource://gre/modules/commonjs/promise/core.js");
> Cu.import("resource:///modules/devtools/EventEmitter.jsm");
>+Cu.import("resource://gre/modules/devtools/Require.jsm");
>+Cu.import("resource:///modules/devtools/gcli.jsm");
>+var util = require('gcli/util');

s/var util/let gcliUtil/

> 
>   /**
>+   * Copy the unique selector of the selected Node to the clipboard.

s/the unique/a unique/

Comment 11

[Paul Rouget [:paul]]

Once the test has been added, can you ask ":jwalker" to review this patch?

Thank you!

Comment 12

[Paul Rouget [:paul]]

(clearing needinfo)

Comment 13

[Paul Rouget [:paul]]

Oh, and can you use this technique to create a patch:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 14

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Paul Rouget [:paul] from comment #13)
> Oh, and can you use this technique to create a patch:
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F

...and by that I believe Paul means to make sure your patch has the metadata (author, comment, etc.) on top, like what you get from 'hg export'.

Comment 15

[Christos Stathis]

Thanks for the quick feedback! I 'll have a test and a correct patch ASAP.

Comment 16

[Christos Stathis]

Attachment: Update previous patch with a test

I hope it 's ok now. I added the test in browser_inspector_menu.js as it seems appropriate.

Comment 17

[Joe Walker [:jwalker]]

Comment on attachment 708813 [details] [diff] [review]
Update previous patch with a test

Review of attachment 708813 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for this.
I've been thinking about the best way to do this, and I think we shouldn't reuse code from GCLI like this. Can I suggest that we copy the code into CssLogic, and then later, I'll move the code into "host.js" so we don't have this duplicated?
Thanks.

Comment 18

[Paul Rouget [:paul]]

FYI, a Selectors.jsm module is coming (bug 831693). That might be a better place.

Comment 19

[Christos Stathis]

Attachment: Update previous patch based on jwalker 's suggestions

If I understood it correctly, I copied the findCssSelector function into CssLogic and called it from there instead of gcli.jsm

Comment 20

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/d4862b10556c

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/d4862b10556c