Closed Bug 1172320 Opened 6 years ago Closed 6 years ago
Browser Content Toolbox should have an accesskey
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
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
Pff, not even having tested before submitting my patch, sorry. Patch updated and works now. Florent
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
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
(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.)
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
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.
You need to log in before you can comment on or make changes to this bug.