Add "Copy Attribute Value" context menu option to elements in the DOM Inspector

VERIFIED FIXED in Firefox 54

Status

P3
enhancement
VERIFIED FIXED
3 years ago
3 months ago

People

(Reporter: nlwillia, Assigned: djmdeveloper060796)

Tracking

({dev-doc-complete, good-first-bug})

unspecified
Firefox 54
dev-doc-complete, good-first-bug

Firefox Tracking Flags

(firefox54 verified, firefox55 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151029151421

Steps to reproduce:

Open the developer tools for a page and select the Inspector tab.  On any element with an attribute, right-click on the attribute value.


Actual results:

The right-click context menu displays with various options related to the element including copying the inner/outer HTML.


Expected results:

The context menu should also include a "Copy Attribute Value" option that would copy the text value of the attribute the mouse was over to the clipboard.

There is limited support for this behavior already with hyperlinks.  If you click on the underlined URl in the href attribute, then the context menu contains link-related options.  However, hrefs are not the only sort of attribute a developer might be interested in copying.  There should be general support for recognizing that the mouse cursor was over an attribute and making the text value of that attribute available to copy.
(Reporter)

Updated

3 years ago
Component: Untriaged → Developer Tools: Inspector
(Reporter)

Updated

3 years ago
OS: Unspecified → Windows 8.1
Hardware: Unspecified → x86_64

Updated

3 years ago
Severity: normal → enhancement

Updated

2 years ago
OS: Windows 8.1 → All
Priority: -- → P3
Hardware: x86_64 → All
Version: 42 Branch → unspecified
This was recently requested via a comment on https://hacks.mozilla.org/2017/01/devtools-what-you-need-to-know
Status: UNCONFIRMED → NEW
Ever confirmed: true
This seems like a good first bug to me.
We have context menus already, we know how to access attributes, and how to access the clipboard. So it's just a matter of wiring all of these things together.

If someone is interested in fixing this, make sure you know how to contribute first: https://developer.mozilla.org/en-US/docs/Tools/Contributing

Once ready to build and run Firefox, you might be interested in these files:

\devtools\client\inspector\inspector.js
especially the _openMenu function inside this file. It dynamically builds the content of the context menu when something is right-clicked in the inspector.
It also calls the _getAttributesSubmenu function to create the content of the "attributes" sub-menu.
I believe the "Copy attribute value" menu should be part of this sub-menu, so the code to show it should be in _getAttributesSubmenu.
The isAttributeClicked boolean is interesting here to enable/disable the menu.
The clipboardHelper object in this file is also useful to copy things to the clipboard.

\devtools\client\inspector\test\browser_inspector_menu-01-sensitivity.js
This is one of our test files. This one will most probably need to be udpated so the ALL_MENU_ITEMS const also contains the new menu item.

\devtools\client\inspector\test\browser_inspector_menu-02-copy-items.js
This is another test file that should be updated. This one tests a number of copy features. Since we're adding a new one here, a new test case should be added to this file too.

Feel free to ask for help on IRC or inside this bug!
https://wiki.mozilla.org/DevTools/GetInvolved#Communication
Keywords: good-first-bug
(Assignee)

Comment 3

2 years ago
@patrick I would like to work on this bug :)
Hi Deepjyoti,

I have assigned the bug to you. Please let us know if you need any assistance.
Assignee: nobody → djmdeveloper060796
Status: NEW → ASSIGNED
(Assignee)

Comment 5

2 years ago
Created attachment 8836445 [details] [diff] [review]
bug1231932.patch

Hi Gabriel, I have added a new sub menu option to attributes which will allow users to copy the value of selected attributes to clipboard. 
This is a test patch, If it is alright I will start working on the tests.
Attachment #8836445 - Flags: review?(gl)
Comment on attachment 8836445 [details] [diff] [review]
bug1231932.patch

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

This looks good so far. It will require the unit tests as described by pbro before it lands.

::: devtools/client/inspector/inspector.js
@@ +1261,4 @@
>        disabled: !isAttributeClicked,
>        click: () => this.onEditAttribute(),
>      }));
> +    attributesSubmenu.append(new MenuItem({

I think this should be moved before Edit Attribute to maintain an alphabetically sorted submenu.

@@ +1263,5 @@
>      }));
> +    attributesSubmenu.append(new MenuItem({
> +      id: "node-menu-copy-attribute",
> +      label: INSPECTOR_L10N.getFormatStr("inspectorCopyAttributeValue.label",
> +                                        isAttributeClicked ? `"${nodeInfo.name}"` : ""),

nodeInfo.name should be the value.

@@ +1800,5 @@
>    /**
> +   * Copy attribute value for node.
> +   * Used for node context menu and shouldn't be called directly.
> +   */
> +  onCopyAttributeValue: function () {

Move this function before onEditAttribute.

::: devtools/client/locales/en-US/inspector.properties
@@ +114,5 @@
> +# LOCALIZATION NOTE (inspectorCopyAttributeValue.label): This is the label of a
> +# sub-menu "Attribute" in the inspector contextual-menu that appears
> +# when the user right-clicks on the attribute of a node in the inspector,
> +# and that allows to copy the value of this attribute to clipboard.
> +inspectorCopyAttributeValue.label=Copy Value %S

s/Copy Value/Copy Attribute Value
Attachment #8836445 - Flags: review?(gl)
(Assignee)

Comment 7

2 years ago
Created attachment 8837999 [details] [diff] [review]
bug1231932.patch

Updated the test files.
Attachment #8836445 - Attachment is obsolete: true
Attachment #8837999 - Flags: review?(gl)

Updated

2 years ago
Attachment #8837999 - Flags: review?(gl) → review+

Comment 8

2 years ago
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fda05fd4c87
Adds Copy Attribute Value context menu to elements in the DOM Inspector; r=gl
Thank you for the patch Deepjyoti!

I went ahead and landed the patch for you. Good work!
(Assignee)

Comment 10

2 years ago
@Gabriel, Thanks :)

I noticed the following in _openMenu function:

The attributes sub menu and the paste sub menu are being created in separate functions, but the copy sub menu is being created in the _openMenu function itself. I think it would be nice if copy sub menu gets its own function too like the other two. I guess it will make the code look more consistent and would reduce the length of _openMenu function.If this idea sounds good then I would love to make the changes by creating a corresponding issue.
(Assignee)

Updated

2 years ago
Flags: needinfo?(gl)
I had to back this out for clipboard test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=77995131&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/ddba1d49b739
Flags: needinfo?(djmdeveloper060796)
(Assignee)

Comment 12

2 years ago
I will update the test file and upload a new patch soon
Flags: needinfo?(djmdeveloper060796)
(In reply to Deepjyoti Mondal from comment #10)
> @Gabriel, Thanks :)
> 
> I noticed the following in _openMenu function:
> 
> The attributes sub menu and the paste sub menu are being created in separate
> functions, but the copy sub menu is being created in the _openMenu function
> itself. I think it would be nice if copy sub menu gets its own function too
> like the other two. I guess it will make the code look more consistent and
> would reduce the length of _openMenu function.If this idea sounds good then
> I would love to make the changes by creating a corresponding issue.

Yes, this sounds good!
Flags: needinfo?(gl)

Updated

2 years ago
Flags: qe-verify+
Created attachment 8838823 [details] [diff] [review]
bug1231932.patch
Attachment #8837999 - Attachment is obsolete: true
Attachment #8838823 - Flags: review+

Updated

2 years ago
Attachment #8838823 - Attachment is patch: true

Comment 15

2 years ago
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80e5378cc1c8
Added copy attribute value context menu to elements in the DOM Inspector; r=gl
(Assignee)

Updated

2 years ago
See Also: → bug 1340762
(Assignee)

Comment 16

2 years ago
@Gabriel Thanks a lot for updating my patch :)

I have created an issue and uploaded a patch for the issue I mentioned in comment 10
Flags: needinfo?(gl)
(In reply to Deepjyoti Mondal from comment #16)
> @Gabriel Thanks a lot for updating my patch :)
> 
> I have created an issue and uploaded a patch for the issue I mentioned in
> comment 10

Np, feel free to flag me for a review in that bug.
Flags: needinfo?(gl)

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80e5378cc1c8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54

Updated

2 years ago
Depends on: 1341586
I have reproduced this bug with Nightly 44.0a1 (2015-10-29) on WIndows 7 , 64 Bit!

This bug's fix is verified with latest Nightly!

Build   ID : 20170301030202
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

[bugday-20170301]
I reproduced this bug using Fx 45.0a1, build ID: 20151211030207, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 54.0a2, build ID:  20170314004020, and Fx 55.0a1, build ID: 20170315030215  on Windows 10 x64 and Mac OS X 10.11.

Cheers!
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
status-firefox55: --- → verified
Flags: qe-verify+
Keywords: dev-doc-needed

Updated

a year ago
Keywords: dev-doc-needed → dev-doc-complete

Updated

3 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.