Closed Bug 1172320 Opened 6 years ago Closed 6 years ago

Browser Content Toolbox should have an accesskey

Categories

(DevTools :: General, defect)

41 Branch
defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: fayolle-florent, Assigned: fayolle-florent)

Details

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150604162752

Steps to reproduce:

I use the browser content toolbox a lot, but it is quite hard to access it.
I like using the accesskeys (like Alt-twe in order to access the Browser Toolbox), and I'd like to be able to do so for the Browser Content Toolbox, like Alt-twx.

Florent
Component: Untriaged → Developer Tools
Attached patch 1172320.patch (obsolete) — Splinter Review
Paul, I am not sure you're the right person to ask a review for that, but I think that could be quite easy to review :) (unless the accesskey choice has to be discussed).

Thanks in advance!

Florent
Attachment #8616456 - Flags: review?(paul)
Attached patch 1172320.patch (obsolete) — Splinter Review
Pff, not even having tested before submitting my patch, sorry. Patch updated and works now.

Florent
Attachment #8616456 - Attachment is obsolete: true
Attachment #8616456 - Flags: review?(paul)
Attachment #8616457 - Flags: review?(paul)
Attachment #8616457 - Flags: review?(paul) → review?(bgrinstead)
Comment on attachment 8616457 [details] [diff] [review]
1172320.patch

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

Seems fine

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +274,5 @@
>  <!-- LOCALIZATION NOTE (browserContentToolboxMenu.label): This is the label for the
>    -  application menu item that opens the browser content toolbox UI in the Tools menu.
>    -  This toolbox allows to debug the chrome of the content process in multiprocess builds.  -->
>  <!ENTITY browserContentToolboxMenu.label     "Browser Content Toolbox">
> +<!ENTITY browserContentToolboxMenu.accesskey     "x">

Nit: can you line up the "x" right below the previous string value?
Attachment #8616457 - Flags: review?(bgrinstead) → review+
Assignee: nobody → fayolle-florent
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached patch 1172320.patchSplinter Review
Done

Florent
Attachment #8616457 - Attachment is obsolete: true
One last question: is a try-push mandatory in all cases? (for this patch, it doesn't seem useful to make a try-push, but maybe the process is strict about it?)

Thanks in advance

Florent
Flags: needinfo?(bgrinstead)
(In reply to fayolle-florent from comment #5)
> One last question: is a try-push mandatory in all cases? (for this patch, it
> doesn't seem useful to make a try-push, but maybe the process is strict
> about it?)
> 
> Thanks in advance
> 
> Florent

I've seen weird unrelated breakage before when changing keyboard shortcuts, but I think in this case you would be safe to request checkin without a full try run.  From https://wiki.mozilla.org/Tree_Rules/Integration#What_are_the_tree_rules_for_integration_repositories.3F:

Integration repositories are no replacement for Try. You still need to test your patches before pushing. (This doesn't mean that you need an all-platforms try run for every patch. But it does mean that you should do enough testing so that you rarely cause red or orange on the integration repository. But breaking it rarely is ok. Never missing a plane means you're spending too much time in airports; never breaking the tree means you're running too many tests before landing.)
Flags: needinfo?(bgrinstead)
Keywords: checkin-needed
Can you please make sure that future patches include your name and email address as well? If you run ./mach mercurial-setup, it'll help you do that.
Sorry, I thought I have already done that, but I'll check that.

Florent
https://hg.mozilla.org/mozilla-central/rev/fdaad6bb56fd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
QA Whiteboard: [good first verify][verify in Aurora only]
I was able to reproduce the issue using Aurora 40.0a2 (2015-06-07) on Win10 Pro x64.

I verified the fix on latest Aurora (2016-09-19) on Win 10 Pro x64 and the shortcut works fine.
On Ubuntu 16.04 LTS x64 and Mac OS X the shortcut is not working.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.