Closed Bug 1158046 Opened 9 years ago Closed 9 years ago

Add access keys to all context menu items in the Network panel

Categories

(DevTools :: Netmonitor, defect)

defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: janx, Assigned: janx)

References

(Depends on 1 open bug)

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(2 files, 1 obsolete file)

Currently, a few context menu items don't have access keys in the Network panel, e.g. when right-clicking on a resource, you can't trigger "Copy as cURL" or "Copy Request Headers" from the keyboard.

We should add access keys to more (maybe all) context menu items so that they can be executed faster.
Blocks: 1158144
Status: NEW → ASSIGNED
Depends on: 1150717
Summary: Add more accessKeys to context menu items in the Network panel → Add access keys to all context menu items in the Network panel
I improved the netmonitor request item context menu a little, by adding access keys for all entries, and using a more sensible order / more separators.

Try (includes dependency patches form bug 1150717): https://treeherder.mozilla.org/#/jobs?repo=try&revision=98f803b2be6a
Attached image contextmenu.png
Here is what the proposed context menu looks like.
Comment on attachment 8602164 [details] [diff] [review]
Add access keys to all context menu items in the Network panel.

Brian, please have a look.
Attachment #8602164 - Flags: review?(bgrinstead)
Comment on attachment 8602164 [details] [diff] [review]
Add access keys to all context menu items in the Network panel.

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

The code changes seem fine to me.  Forwarding the hard part (deciding whether the accesskey choices and menu organization is OK) to jsantell.  Jordan, this screenshots illustrates the updated context menu: https://bug1158046.bugzilla.mozilla.org/attachment.cgi?id=8602175
Attachment #8602164 - Flags: review?(jsantell)
Attachment #8602164 - Flags: review?(bgrinstead)
Attachment #8602164 - Flags: review+
Comment on attachment 8602164 [details] [diff] [review]
Add access keys to all context menu items in the Network panel.

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

All looks good to me! How are these access keys shown anywhere? How would someone know "U" copies the URL?

::: browser/devtools/netmonitor/netmonitor-view.js
@@ +1861,5 @@
>        !selectedItem.attachment.responseContent ||
>        !selectedItem.attachment.responseContent.content.mimeType.includes("image/");
>  
> +    let separators = $all(".request-menu-context-separator");
> +    [].forEach.call(separators, separator => separator.hidden = !selectedItem);

Can also directly use the `Array.forEach` static method for this
Attachment #8602164 - Flags: review?(jsantell) → review+
Thanks!

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #5)
> All looks good to me! How are these access keys shown anywhere? How would
> someone know "U" copies the URL?

The usual way to show access keys is to underline their letters in each option: https://bug1158046.bugzilla.mozilla.org/attachment.cgi?id=8602175 

> > +    [].forEach.call(separators, separator => separator.hidden = !selectedItem);
> 
> Can also directly use the `Array.forEach` static method for this

I believe that's the same prototype method that `[].forEach` accesses.
> > > +    [].forEach.call(separators, separator => separator.hidden = !selectedItem);
> > 
> > Can also directly use the `Array.forEach` static method for this
> 
> I believe that's the same prototype method that `[].forEach` accesses.

Well, actually not. Fixed.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98f803b2be6a

Sheriffs, please apply this patch *after* those from bug 1150717.
Attachment #8602164 - Attachment is obsolete: true
Attachment #8602208 - Flags: review+
As mentioned in comment 7, please land this patch *after* those from bug 1150717. Thanks!
Keywords: checkin-needed
Whiteboard: [devedition-40][difficulty=easy] → [devedition-40][difficulty=easy][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/91e5abde70fe
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [devedition-40][difficulty=easy][fixed-in-fx-team] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
QA Whiteboard: [good first verify]
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: