Closed Bug 1316855 Opened 3 years ago Closed 3 years ago

Tooltip of "Toolbox Buttons" should show their shortcut

Categories

(DevTools :: Framework, enhancement, P3)

50 Branch
enhancement

Tracking

(firefox53 fixed)

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: nachtigall, Assigned: brennan.brisad, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

The "Toolbox Buttons" do not have their shortcuts in the mouseover tooltip. 

The most often shortcuts I use are ESC ("Toggle split console") and Ctrl+Shift+M ("Responsive Design Mode"). I think I learnt one by accident, the other one from a colleague - after month of usage. 

I think the tooltips of "Toolbox Buttons" should also show their shortcuts, like the tooltips of the Tools (Inspector, Console, etc.) already do. 

For reference of what's available, probably there's even more shortcuts that could be shown in tooltips: https://developer.mozilla.org/en-US/docs/Tools/Keyboard_shortcuts
Seems like a good suggestion to me, thanks for filing!
Severity: normal → enhancement
Priority: -- → P3
Wouldn't this make for a "good first bug" so whoever comes around can grab it? Just attaching the Shortcut codes to the tooltips looks simple to me... 

My experience with bugs that are "P3" *and* "enhancement" *and not* listed on http://firefox-dev.tools/ is that these will be buried in the depth of bugzilla and not get fixed for months/years.
Flags: needinfo?(jryans)
See Also: → 892188
Sure, let's give it a try.

To get started with the code base, check out https://wiki.mozilla.org/DevTools/Hacking and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build.

Here's a search listing the lines that pull in the tooltip for each button:

http://searchfox.org/mozilla-central/search?q=tooltipText%3A&case=true&regexp=false&path=devtools

Here's the file which contains the actual strings themselves:

http://searchfox.org/mozilla-central/source/devtools/shared/locales/en-US/gclicommands.properties

Here's a list of DevTools shortcuts:

https://developer.mozilla.org/en-US/docs/Tools/Keyboard_shortcuts

I believe "Split console" and "Responsive Design Mode" may actually be the only two that currently have shortcuts defined.  Shortcuts can differ per platform, so check the list carefully.

I would suggest changing from `l10n.lookup` to `l10n.lookupFormat` and passing in the shortcut text.  The string will need to be changed to accept a value.  When change string values, you need to change the string ID as well[1].

If you have questions, check the "need more info" box on this page and choose "mentor".

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Updating_Entity_Names
Mentor: jryans
Flags: needinfo?(jryans)
Keywords: good-first-bug
Attached patch tooltips.patch (obsolete) — Splinter Review
Hi, I made a patch which I'm attaching for feedback.

One shortcut seemed to be embedded in the code in `jsterm.js` while the other one I found in another properties file, `menu.properties`. So I hardcoded them for now, but please let me know if there's a better way to solve it.
Attachment #8814625 - Flags: feedback?(jryans)
Comment on attachment 8814625 [details] [diff] [review]
tooltips.patch

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

Great, these changes look good to me!

Can you repost your changes as an hg patch?  If you are using git locally, here are some tips[1] to do this.  When you repost, set the new attachment for review with my email and also mark the old one as obsolete.

Format the commit message as shown on that page and use "r=jryans" at the end of the line.

Thanks for looking into this!

[1]: https://developer.mozilla.org/en-US/docs/Tools/Contributing#Creating_a_patch_to_check_in
Attachment #8814625 - Flags: feedback?(jryans) → feedback+
Assignee: nobody → brennan.brisad
Status: NEW → ASSIGNED
Attached patch Bug1316855.patchSplinter Review
Attachment #8814625 - Attachment is obsolete: true
Attachment #8815451 - Flags: review?(jryans)
Comment on attachment 8815451 [details] [diff] [review]
Bug1316855.patch

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

Great, this looks good to me.  Thanks for working on it!

I pushed this to our CI system to make sure tests are passing:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=982831c7c57e2ff4d31599b723584179d520b578

Assuming that looks good, we can set the "checkin-needed" keyword for a sheriff to land the patch.  The bug will get resolved once it has been merged to mozilla-central.

If you're interested in more DevTools bugs to work on, check out http://firefox-dev.tools for more ideas!
Attachment #8815451 - Flags: review?(jryans) → review+
Thank you for the feedback!

I'm happy to be able to help out, I'll definitely go look at http://firefox-dev.tools for more.
Looks ready to land.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d582b93a3785
Tooltip of "Toolbox Buttons" should show their shortcut. r=jryans
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d582b93a3785
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
I have reproduced this bug with Nightly 52.0a1 (2016-11-11) (64-bit) on windows 7 , 64 Bit ! 

This bug's fix is verified with latest Nightly

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

[bugday-20161207]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.