It's very hard to copy headers and values

ASSIGNED
Assigned to

Status

enhancement
P3
normal
ASSIGNED
a year ago
2 months ago

People

(Reporter: peterbe, Assigned: tanhengyeow, Mentored, NeedInfo)

Tracking

(Blocks 2 bugs, {good-first-bug})

58 Branch
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

User Story

See comment #8 for detailed description of what should be done here

Attachments

(3 attachments, 7 obsolete attachments)

(Reporter)

Description

a year ago
See attached screenshot. I want to copy that to my clipboard. Ideally, I'd like to be able to Cmd-C after I have highlighted the row. Second-ideally to be able to right-click on the row and select Copy. (but there is no right-click menu). 

Also, it would be cool to be able to get the key AND value, e.g. "If-modified-since: Sat, 01 Jan ....", into the clipboard.
Thanks for the report and yes Copy to the clipboard should really be better.

Honza
Priority: -- → P3

Comment 2

a year ago
I totally agree, it is very hard. This is especially annoying when trying to copy these long CSP headers, which you otherwise cannot read in the browser.

When I click on the right side, it actually selects something or so, but not the full value. To copy the value I first have to right-click -> "Mark all" and then Ctrl+C or right-click -> Copy.

Firefox 59
Linux
What would be the ideal UX for you here?

Honza
(Reporter)

Comment 4

a year ago
(In reply to Jan Honza Odvarko [:Honza] from comment #3)
> What would be the ideal UX for you here?
> 
> Honza

I posted some ideal UX ideas in the bug description.

Comment 5

a year ago
Indeed, at the left side -> Ctrl+C would copy all. At the right side would maybe only copy the value of the header.
The CSS rules panel in the inspector has a few copy options when you right-click. If you click on a property, you can either copy the name, the value, or both (formatted like name:value;). I think something like this would be very helpful.

Also, and this probably belongs in another bug, when clicking on one line it might be good to show the entire value, with line wrapping if necessary. Sometimes you just want to check something quickly, not copy the value.

Honza, the right-click context menu idea seem fairly easy to implement, using the Menu API we use in other places already. Do you mind describing the necessary steps for solving this bug? I'm sure someone could be interested in fixing it this way.
Flags: needinfo?(odvarko)
Blocking the gcli removal bug because gcli has a `security csp` command that makes it easier to see the csp headers than by using the network monitor.
Blocks: gcli-removal
Summary: It's very hard copy headers and values → It's very hard to copy headers and values
Detailed instructions for anyone interested in this bug:

1) The header values are currently cropped and it makes it hard to read in case they are longer than 60 characters.
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/HeadersPanel.js#155

We should remove that limit (there is no limit for Cookies or Params either). We might want to introduce a lot bigger limit e.g. 1024..?

2) Clicking on header value selects the row and also opens an input box that contains the value (you might notice light blue border around the value - that's input box border). This has been done to make it easier to copy the value (from an input box), but the opposite happened. The text in the input isn't selected by default and so, right clicking on it and executing "Copy" doesn't work. The user needs to execute "Select All" first. No good UI.

Clicking on the value should behave the same as clicking on the name - i.e. The entire HTTP header name + value row is selected.

The inputbox features is enabled using `enableInput` prop:
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/PropertiesView.js#219

... it should be removed.

3) We need to display a context menu when the user clicks on selected header row.

The menu should contain the following menu actions:

* Copy Header
* Copy Header Name
* Copy Header Value

(we might discuss the labels)

Strings should be localized in this file:
devtools/client/locales/en-US/netmonitor.properties

Clicking on header name or value should open the same menu.


4) To handle click we need to implement a handler

- We need to implement a new function in HeadersPanel: getContextMenuItems and pass it as a prop to PropertiesView here:
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/HeadersPanel.js#303

The Header object 'object'  should be passed as an argument into the getContextMenuItems method.

- The PropertiesView needs to pass new onClickRow handler to TreeView component here:
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/PropertiesView.js#209

- Here is the TreeView.onClickRow prop description
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/shared/components/tree/TreeView.js#119

- The PropertiesView needs to open a context menu in the onClickRow handler. See an example of the context menu displayed when the user clicks on the list of requests:
https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/RequestListContent.js#238

- The onClickRow handler should create instance of the ContextMenu, but the list of menu items should be returned by HeaderPanel.getContentMenuItems.


- The CokiesPanel and ParamsPanel should do similar things - implement their own *.getContentMenuItems handler that returns menu items for Cookies resp. Params related panels (getContentMenuItems will get Cookie resp. Param object) This should probably be done as part of different bug.


Honza
Mentor: odvarko
Flags: needinfo?(odvarko)
Keywords: good-first-bug
User Story: (updated)
Hey, I'd like to tackle this bug! Could you assign it to me?
(In reply to Connor Bach [:connorb] from comment #9)
> Hey, I'd like to tackle this bug! Could you assign it to me?
Done!

Honza
Assignee: nobody → cbach120
Status: NEW → ASSIGNED
(In reply to Jan Honza Odvarko [:Honza] from comment #10)
> (In reply to Connor Bach [:connorb] from comment #9)
> > Hey, I'd like to tackle this bug! Could you assign it to me?
> Done!
> 
> Honza

I made the first two changes and they are both working locally. Should I implement the context menu in the HeadersPanel.js file? I looked around for documentation on creating context menus but didn't see anything too applicable. Would you be able to point me in the right direction? Thanks!
(In reply to Connor Bach [:connorb] from comment #11)
> I made the first two changes and they are both working locally. Should I
> implement the context menu in the HeadersPanel.js file? I looked around for
> documentation on creating context menus but didn't see anything too
> applicable. Would you be able to point me in the right direction? Thanks!

See my comment above, good example is directly in Net monitor code base.
Search in RequestListContent and RequestListContextMenu

https://searchfox.org/mozilla-central/rev/028cd8342899357d80fafba2d528a0fd5819e316/devtools/client/netmonitor/src/components/RequestListContent.js#238

Since there might be at least three menus with different actions in the future:
- one for the Headers panel (implemented in this bug)
- one for the Cookies panel (different bug)
- one for the Params panel (different bug)
- and perhaps other panels....

... we should do it a big generic. The code that shows the menu is there just once in PropertiesView and every panel provides its own set of items through getContextMenuItems. This method can be passed as a prop into PropertiesView.

So, the HeaderPanel.getContextMenuItems() could look like something as follows:

getContextMenuItems(header) {
  let items = [];

  items.push({
    id: "headers-panel-copy",
    label: L10N.getStr("netmonitor.headers.copy"),
    accesskey: L10N.getStr("netmonitor.headers.copy.accesskey"),
    visible: true,
    click: () => this.copyHeader(header),
  });

  // ...
  
  return items;
}

And there should be one implementation of the menu itself in the PropertiesView
(the name could be onContextMenu, sounds a bit better than onClickRow)

loader.lazyRequireGetter(this, "showMenu", "devtools/client/netmonitor/src/utils/menu", true);

onContextMenu(event) {
  event.preventDefault();

  let { getContextMenuItems, object } = this.props;
  let items = getContextMenuItems(object);

  showMenu(event, items);
}

Honza
@Connor: hi, any progress on this bug? Anything I can help with?

Honza
Flags: needinfo?(cbach120)
@Honza: I added the code for the context menu in HeadersPanel.js as well as the code for generating the contextMenu in PropertiesView.js

The context menu does not appear on click unfortunately, I think it might be an issue with how I am passing the getContextMenuItems method to PropertiesView.

In HeadersPanel.js I have this line:
          onContextMenu: this.getContextMenuItems,

In PropertiesView.js I have this:
            onClickRow: this.onContextMenu,

Let me know if there is an issue with how I am passing the functions.

Appreciate your help!

Connor
Flags: needinfo?(cbach120)
Connor, can you attach your patch, so I can try it on my machine?

Honza
Flags: needinfo?(cbach120)
Pass onContextMenu from PropertiesView.js to TreeView.js
(In reply to Connor Bach [:connorb] from comment #16)
> Created attachment 8965360 [details] [diff] [review]
> Bug <> - Pass getContextMenuItems to PropertiesView.js
> 
> Pass onContextMenu from PropertiesView.js to TreeView.js

The patch contains only one change:

> onContextMenu: this.getContextMenuItems,

Is that all?

Also, please always set "Need more info" for me (see at the bottom of this page), so your question appears on my TODO list. Otherwise it can be easily lost in bugmail noise, thanks.

Honza
@Connor: Hi, are you working on this bug?
Anything I can help with?

Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #18)
> @Connor: Hi, are you working on this bug?
> Anything I can help with?
> 
> Honza

@Honza, Yes I am still working on the bug.

  /******** HeadersPanel.js **************/

  getContextMenuItems(header) {
    let items = [];

    items.push({
      id: "headers-panel-copy",
      label: L10N.getStr("netmonitor.headers.copy"),
      accesskey: L10N.getStr("netmonitor.headers.copy.accesskey"),
      visible: true,
      click: () => this.copyHeader(header),
    });
  }

  Line 341, returning to PropertiesView has this line.
  onContextMenu: this.getContextMenuItems,



  /********* PropertiesView.js *************/

  loader.lazyGetter(this, "showMenu", "devtools/client/netmonitor/src/utils/menu", true);

    onContextMenu(event) {
    event.preventDefault();
    
    let { getContextMenuItems, object } = this.props;
    let items = getContextMenuItems(object);

    showMenu(event, items);
  }

    onClickRow: this.onContextMenu,

Not sure how to post a patch that contains more than one commit of changes but here are the changes I made.
I added the code for creating the context menu in HeadersPanel.js and then returned it to PropertiesView.js
In PropertiesView.js I wrote an event that should show the context menu, and I return that to the TreeView at the bottom of the file. 

Not sure if I am still missing some code or if I need to change the way I am passing the events/arguments between files. Appreciate your help!

Connor
Flags: needinfo?(cbach120) → needinfo?(odvarko)
(In reply to Connor Bach [:connorb] from comment #19)
> Not sure how to post a patch that contains more than one commit of changes
> but here are the changes I made.

The code you posted looks like it's going in the right direction. But, let's first see how to create a patch, so we can properly communicate through reviews and patches that can be easily applied on top of the mozilla-central (m-c) source repository.

If you use HG, you can:
hg diff > copy-headers.patch

If you use GIT (cinnabar), you can:
git diff > copy-headers.patch

Consequently, you can attach the result file to this bug (see `Attach File` at the top of this page).

---

You should also read this page:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Honza
Flags: needinfo?(odvarko) → needinfo?(cbach120)
(In reply to Jan Honza Odvarko [:Honza] from comment #20)
> (In reply to Connor Bach [:connorb] from comment #19)
> > Not sure how to post a patch that contains more than one commit of changes
> > but here are the changes I made.
> 
> The code you posted looks like it's going in the right direction. But, let's
> first see how to create a patch, so we can properly communicate through
> reviews and patches that can be easily applied on top of the mozilla-central
> (m-c) source repository.
> 
> If you use HG, you can:
> hg diff > copy-headers.patch
> 
> If you use GIT (cinnabar), you can:
> git diff > copy-headers.patch
> 
> Consequently, you can attach the result file to this bug (see `Attach File`
> at the top of this page).
> 
> ---
> 
> You should also read this page:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> How_to_Submit_a_Patch
> 
> Honza

When I run "hg diff > copy-headers.patch" it seems to generate a blank file.
"hg log" shows several commits that I have made but hg diff doesn't seem to bring them into the patch.
Looking through the MDN docs the hg bzexport only seems to create a patch from my most recent commit.
Any way to create a patch with multiple commits?
Flags: needinfo?(cbach120) → needinfo?(odvarko)
(In reply to Connor Bach [:connorb] from comment #21)
> When I run "hg diff > copy-headers.patch" it seems to generate a blank file.
> "hg log" shows several commits that I have made but hg diff doesn't seem to
> bring them into the patch.
Yes, because diff only shows changes in working directory.

> Looking through the MDN docs the hg bzexport only seems to create a patch
> from my most recent commit.
> Any way to create a patch with multiple commits?

You might try 'hg export <tag|revision number|changeset ID>'

And more commits:
http://www.redmine.org/projects/redmine/wiki/How_to_create_patch_series_on_Mercurial_and_Git

Another example, exporting the latest:
hg export tip

You might also use hg queues or bookmarks, read more on this page:
https://developer.mozilla.org/en-US/docs/Mozilla/Mercurial/Queues

As far as this issue is concerned, I think that it's simple enough that one patch should be sufficient.

Honza
Flags: needinfo?(odvarko)
Posted patch copymenu.patch (obsolete) — Splinter Review
Thanks for your help with the version control. Using hg export -r START_CHANGESET:END_CHANGESET seemed to work. This patch file should include all the changes I made up to this point. Let me know if you have any ideas about where to go from here!

Connor
Flags: needinfo?(odvarko)
Comment on attachment 8968895 [details] [diff] [review]
copymenu.patch

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

> L10.N.getStr("netmonitor.headers.copy")
Should be `L10N.getStr`

The browser console is showing:
TypeError: event.preventDefault is not a function PropertiesView.js:115:5

There are three issues with the `onContextMenu` method.
1) You need to bind it since it's event handler, check out the PropertiesView.constructor and see how other event handlers. You need to do something like: this.onContextMenu = this.onContextMenu.bind(this);
2) The first parameter of `onContextMenu` is `nodePath` the second is `event`: onContextMenu(nodePath, event)
3) You are passing `HeadersPanel.getContextMenuItems` into Properties view as 'onContextMenu' property, but using it as `getContextMenuItems`. The prop name should be `getContextMenuItems`

I don't see "netmonitor.headers.copy" string to be defined in netmonitor.properties file
You don't have to define `accesskey` for the menu items, just remove it.

You need to implement all three copy items
copyHeader
copyHeaderValue
copyHeaderName

In PropertieView, you need to include `showMenu` using lazyRequireGetter:
loader.lazyRequireGetter(this, "showMenu", "devtools/client/netmonitor/src/utils/menu", true);

Again, you need to bind PropertiesView.onContextMenu in the constructor.

The TreeView's onClickRow is called for right click not left click, this needs to be fixed too. But, we can do it as soon as all the other stuff is fixed. For now, you need to right click when testing the code.

---

Generating patch from more commits breaks the review system (Splinter) in Bugzilla, so I can't properly review it (and comment individual pieces of the patch inline). Please make sure to work with one commit only and generate the patch from it (you don't even have to commit anything into your local repo, just can just generate patch from changes in working dir)

Thanks,
Honza

::: devtools/client/netmonitor/src/components/PropertiesView.js
@@ +216,3 @@
>              decorator: decorator || {
>                getRowClass: (rowObject) => this.getRowClass(rowObject, sectionNames),
>              },

nit: space between comma and "showMenu"
Please see my comments above...

Don't forget to mark the previous patch as obsolete.

Click 'Details' link next to the patch name, consequently click "Edit Details" and check "Obsolete" checkbox.

Honza
Flags: needinfo?(odvarko) → needinfo?(cbach120)
Attachment #8968895 - Attachment is obsolete: true
Flags: needinfo?(cbach120)
Posted patch copyheaders.patch (obsolete) — Splinter Review
The context menu is now appearing on click with all three menu items! I think I just need to implement the actual copy functions in headerspanel.js now. Any guidance on this?

Thanks!

Connor
Flags: needinfo?(odvarko)
(In reply to Connor Bach [:connorb] from comment #26)
> The context menu is now appearing on click with all three menu items! I
> think I just need to implement the actual copy functions in headerspanel.js
> now. Any guidance on this?

You can look at this place:
https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/devtools/client/netmonitor/src/har/har-exporter.js#117

Just use clipboardHelper:
const clipboardHelper = require("devtools/shared/platform/clipboard");


...and copyString method:
clipboardHelper.copyString(headerValue);

Honza
Flags: needinfo?(odvarko)
I think I am pretty close to getting this bug wrapped up, running into a problem with getting the correct header value to be copied. Currently the event that is passed into the getContextMenu function contains the information for every header in the network panel:

console.log: "header: " ({'Request headers (525 B)':{headers:[{name:"Accept", value:"*/*"}, {name:"Accept-Encoding", value:"gzip, deflate, br"}, {name:"Accept-Language", value:"en-US,en;q=0.5"}, {name:"Connection", value:"keep-alive"}, {name:"Cookie", value:"1P_JAR=2018-05-08-23; NID=128=bHrl652m7h3R19kJj9zGSgLUP26Zmexu4sP0aWiYzrr-TmMUZxlwMuOKsn32emRZZPpxZBvt_JBvv33x1MN8T9aXAvFX2Rs6SNdhgy1YKGmZSn0ntGqd1wA921UEwfuZJ3z63dI; OGP=-5061451:"}, {name:"Host", value:"www.google.com"}, {name:"Referer", value:"https://www.google.com/"}, {name:"User-Agent", value:"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:61.0) Gecko/20100101 Firefox/61.0"}]}})


I only want to copy the one at the location of the mouse click however. Is there a way to determine which header was clicked on so that I can copy the correct value to the clipboard? Thanks!

- Connor
Flags: needinfo?(odvarko)
@Connor: can you attach the latest version of the patch, so I can try it out on my machine?
I can't apply the currently attached patch on the latest HEAD. It needs to be rebased.

Honza
Flags: needinfo?(odvarko) → needinfo?(cbach120)
Posted patch Copy exact header value patch (obsolete) — Splinter Review
This allows for copying the entire header information. Not sure how to get into each individual header to copy the item that was clicked on.
Attachment #8970894 - Attachment is obsolete: true
Flags: needinfo?(cbach120) → needinfo?(odvarko)
Posted patch copy-headers.patch (obsolete) — Splinter Review
@Connor,
Since the implementation was spread across multiple patches (some of them obsolete and some of them needed rebasing on top of m-c) I merged all together and updated.

It was very close, just the following missing:

- Support for context menu event in TreeView and TreeRow components
- Passing the header object into `getContextMenuItems` (per comment #30)

Missing things:
- Implement similar support for Cookies and Params panels (as mentioned in comment #8)
- Automated tests

You might take a look at browser_net_copy_headers.js or browser_net_copy_params.js to learn how to implement it.

It would be also great to append some comments through the new code...

Tests might be implemented in separate patch, but the implementation of the feature itself should stay in one patch. Please make sure to mark old patches as obsolete if you are attaching new version.


Thanks for helping with this one!

Honza
Flags: needinfo?(odvarko)
Attachment #8965360 - Attachment is obsolete: true
Attachment #8981189 - Attachment is obsolete: true

Updated

10 months ago
Product: Firefox → DevTools

Updated

10 months ago
Duplicate of this bug: 1469218
No longer blocks: gcli-removal
I have created a new bug for the security csp issue, since it seems unrelated to this bug for now: https://bugzilla.mozilla.org/show_bug.cgi?id=1483491
Assignee: cbach120 → nobody
Status: ASSIGNED → NEW

Comment 34

7 months ago
Hi, I'm interested in working on this. Could u assign it to me?
Flags: needinfo?(odvarko)
Hi Aanchal,
assigned to you!

Honza
Assignee: nobody → aanchal120btcse16
Status: NEW → ASSIGNED
Flags: needinfo?(odvarko)

Comment 36

7 months ago
Posted patch copyHeaders.patch (obsolete) — Splinter Review
I've made changes as per the instructions(mentioned in comment #8). The context menu appears but the header values are not getting copied to clipboard.
Could u please help me proceed further? 
Also, how should we handle this console.error: "Tried to send a 'tabDetached' event on an already destroyed actor 'frameTarget'"?
Thanks!
Flags: needinfo?(odvarko)
Comment on attachment 9013528 [details] [diff] [review]
copyHeaders.patch

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

Thanks for working on this!

Please see my inline comments

Also, you need to pass `event.currentTarget` into the `getContextMenuItems` so when menu-actions are executed you can access the underlying clicked element and get the textContent (to copy to the clipboard)

Something like:

let headerName = currentTarget.querySelector(".treeLabelCell").textContent;
let headerValue = currentTarget.querySelector(".treeValueCell").textContent;

You should use The Browser Toolbox to inspect the UI and figure out the right CSS selectors.
https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Honza

::: devtools/client/netmonitor/src/components/CookiesPanel.js
@@ +98,5 @@
> +      visible: true,
> +      click: () => copyString(arr.value),
> +    });
> +  return items;
> +  }

Cookies panel needs its own set of menu items for copying cookies.

Let's introduce:

1) Copy Cookie
2) Copy Cookie Name
3) Copy Cookie Value

All items need to be also localized.

::: devtools/client/netmonitor/src/components/HeadersPanel.js
@@ +125,5 @@
>    }
>  
> +  getContextMenuItems(headers) {
> +    let items = [];
> +  

Remove the trailing spaces on this empty line

@@ +126,5 @@
>  
> +  getContextMenuItems(headers) {
> +    let items = [];
> +  
> +      items.push({

Fix the indentation on this line

@@ +144,5 @@
> +      label: L10N.getStr("netmonitor.headers.copyName"),
> +      visible: true,
> +      click: () => clipboardHelper.copyString(headers.value),
> +    });
> +  return items;

Fix indentation on this line

::: devtools/client/netmonitor/src/components/ParamsPanel.js
@@ +112,5 @@
> +      visible: true,
> +      click: () => copyString(arr.value),
> +    });
> +  return items;
> +  }

The Param panel needs its own set of menu item actions:

Let's introduce:

1) Copy Param
2) Copy Param Name
3) Copy Param Value

All items need to be also localized.

::: devtools/client/netmonitor/src/components/PropertiesView.js
@@ +112,4 @@
>  
>      const jsonString = JSON.stringify({ [name]: value }).toLowerCase();
>      return jsonString.includes(filterText.toLowerCase());
> +  }    

Remove trailing spaces on this line

@@ +118,5 @@
> +    event.preventDefault();
> +
> +    let { getContextMenuItems, object } = this.props;
> +
> +    let items = getContextMenuItems(object.name);

You should pass `event.currentTarget` in the the `getContextMenuItems` so, when individual items are executed, they can dig the text-to-copy from the DOM.
Also, please mark the old patch as obsolete (attachment -> Edit -> Edit Details -> Obsolete

Honza
Flags: needinfo?(odvarko)

Updated

7 months ago
Attachment #8983004 - Attachment is obsolete: true

Comment 39

7 months ago
Posted patch copyHeadersValue.patch (obsolete) — Splinter Review
The information you provided was very helpful. Thanks!
I've tried to implement a right-click context menu for copying header and header values.
Attachment #9013528 - Attachment is obsolete: true
Attachment #9014688 - Flags: review?(odvarko)

Updated

7 months ago
Attachment #9014688 - Attachment is obsolete: true
Attachment #9014688 - Flags: review?(odvarko)

Comment 40

7 months ago
Sorry for attaching a wrong patch earlier. This is the correct one.
Attachment #9014721 - Flags: review?(odvarko)
Comment on attachment 9014721 [details] [diff] [review]
copyHeaderValue.patch

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

Thanks for the patch, works like a charm!

Just a few nit comments in inline.

This bug also needs an automated test.

You might see these tests for inspiration
browser_net_header-docs.js
browser_net_copy_headers.js

Honza

::: devtools/client/netmonitor/src/components/HeadersPanel.js
@@ +122,4 @@
>      });
>    }
>  
> +  getContextMenuItems(currentTarget){

missing space between the function and curly bracket. It should be:

getContextMenuItems(currentTarget) {

(and similarly on other places in this patch)

@@ +124,5 @@
>  
> +  getContextMenuItems(currentTarget){
> +    let headerName = currentTarget.querySelector(".treeLabelCell").textContent;
> +    let headerValue = currentTarget.querySelector(".treeValueCell").textContent;
> +    let items = [];

Use const for these vars

(and similarly on other places in this patch)

@@ +183,4 @@
>            // Force StringRep to crop the text everytime
>            member: Object.assign({}, member, { open: false }),
>            mode: MODE.TINY,
> +          cropLimit: 1024,

Why is this changed?
Attachment #9014721 - Flags: review?(odvarko)

Comment 42

7 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #41)

> @@ +183,4 @@
> >            // Force StringRep to crop the text everytime
> >            member: Object.assign({}, member, { open: false }),
> >            mode: MODE.TINY,
> > +          cropLimit: 1024,
> 
> Why is this changed?

The header values are difficult to read if they are longer than 60 characters. So I introduced a bigger limit of 1024 as suggested in comment #8.
QA Contact: odvarko
(In reply to Aanchal Agarwal from comment #42)
> (In reply to Jan Honza Odvarko [:Honza] from comment #41)
> 
> > @@ +183,4 @@
> > >            // Force StringRep to crop the text everytime
> > >            member: Object.assign({}, member, { open: false }),
> > >            mode: MODE.TINY,
> > > +          cropLimit: 1024,
> > 
> > Why is this changed?
> 
> The header values are difficult to read if they are longer than 60
> characters. So I introduced a bigger limit of 1024 as suggested in comment
> #8.
Ok, good.

Honza
Assignee: aanchal120btcse16 → nobody
Status: ASSIGNED → NEW
Assignee: nobody → aanchal120btcse16
Status: NEW → ASSIGNED
QA Contact: odvarko
A few tips:

1) Running a test locally
./mach test devtools/client/netmonitor/test/browser_net_header-docs.js

2) Verifying a test & running with no UI
./mach test devtools/client/netmonitor/test/browser_net_header-docs.js --verify --headless

--verify : running in different modes many times.
--headless : no UI, so you can continue using your machine (and don't worry about e.g. the focus).

3) MDN
https://developer.mozilla.org/en-US/docs/Mozilla/QA/Running_automated_tests

Honza
Any progress with this one?
Can I help somehow?

Honza
Flags: needinfo?(aanchal120btcse16)

Comment 47

6 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #46)
> Any progress with this one?
> Can I help somehow?
> 
> Honza

I'm extremely sorry. I was on vacation and couldn't make much progress. I'll resume working on this today.

Updated

6 months ago
Flags: needinfo?(aanchal120btcse16)

Comment 48

6 months ago
I'm facing difficulty in creating the tests.
TypeError: monitor.panelWin.parent.document.querySelector(...) is null; can't access its "click" property
The css selector ".treeLabelCell" isn't returning the element.
Could u please help me proceed further?
Flags: needinfo?(odvarko)
Sorry for the delay.

I tried to apply your patch, but I am seeing conflict in the latest m-c

applying copyHeaderValue.patch
patching file devtools/client/netmonitor/src/components/PropertiesView.js
Hunk #2 succeeded at 65 with fuzz 2 (offset 0 lines).
patching file devtools/client/shared/components/tree/TreeRow.js
Hunk #1 succeeded at 51 with fuzz 2 (offset 0 lines).
Hunk #2 succeeded at 129 with fuzz 2 (offset 0 lines).
patching file devtools/client/shared/components/tree/TreeView.js
Hunk #4 FAILED at 501
1 out of 4 hunks FAILED -- saving rejects to file devtools/client/shared/components/tree/TreeView.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh copyHeaderValue.patch


Can you please make sure both patches are up to date?

In order to download the latest source, execute the following in your source dir:

hg pull -u

Thanks!
Honza
Flags: needinfo?(odvarko)
(Assignee)

Comment 50

2 months ago

Hi Aanchal Agarwal

Are you still working on this? Do you require any assistance? :)

Flags: needinfo?(aanchal120btcse16)

Comment 51

2 months ago

Hey, can I work on this issue if it is still open?

Flags: needinfo?(odvarko)
(Assignee)

Comment 52

2 months ago

Hi Saumya

Apologies, I'm in the midst of tying up the loose ends for this patch :) Do you mind working on other bugs?

Flags: needinfo?(saumya15172)

Comment 53

2 months ago

No problem.:)

Flags: needinfo?(saumya15172)
Assignee: aanchal120btcse16 → E0032242
Flags: needinfo?(odvarko)
You need to log in before you can comment on or make changes to this bug.