780073 - Add keyboard shortcut tooltips across the debugger UI

Status: RESOLVED DUPLICATE of bug 785889

(DevTools :: Debugger, defect)

Description

[Victor Porof [:vporof][:vp]]

They're totally undiscoverable right now, and we're pretty original in choosing them :) Adding some of the keys as tooltips should help.

Comment 1

[Abhishek Potnis [:avp]]

Hi Victor,

I am interested in working on this bug. Could you please help me on getting started 
with this bug.

Thanks.

Comment 2

[Victor Porof [:vporof][:vp]]

(In reply to Abhishek Potnis [:coolcoder91] from comment #1)
> Hi Victor,
> 
> I am interested in working on this bug. Could you please help me on getting
> started 
> with this bug.
> 
> Thanks.

Hey cool coder ;)
Thanks for the interest! You'll need to modify the values of the "tooltiptext" attribute on some elements. For now, we need to do this for the resume, step over, step in and step-out buttons.

You'll find these buttons defined in browser/devtools/debugger.xul. It's as easy as checking what the value of the "tooltiptext" attribute is, and modify the corresponding string in browser/locales/en-US/chrome/browser/devtools/debugger.dtd. 

The keyset with the id "threadStateKeys" (from debugger.xul) shows you what the keyboard shortcuts are for these buttons. Simply append those to the corresponding strings in debugger.dtd.

Luckily you won't need to write any tests for this patch :)

Comment 3

[Abhishek Potnis [:avp]]

Attachment: edited the debugger.dtd file

I have edited the file debugger.dtd as suggested by you, but I did not find any
String to edit for the resume button.

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 651163 [details] [diff] [review]
edited the debugger.dtd file

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

Good start.
Yeah, for the play/resume button we add those strings dynamically, so you'll need to edit browser/locales/en-US/chrome/browser/devtools/debugger.properties, specifically pauseTooltip and resumeTooltip.

::: browser/locales/en-US/chrome/browser/devtools/debugger.dtd
@@ +45,4 @@
>  
>  <!-- LOCALIZATION NOTE (debuggerUI.stepOutButton): This is the tooltip for the
>    -  button that steps out of a function call. -->
> +<!ENTITY debuggerUI.stepOutButton.tooltip    "Step Out		F8">

Please don't add so much spacing between the text and shortcut. Something like "Step Out (F8)" should do the trick".

Comment 5

[Abhishek Potnis [:avp]]

Attachment: removed the extra spaces from debugger.dtd

Comment 6

[Abhishek Potnis [:avp]]

Attachment: edited debugger.properties

Comment 7

[Victor Porof [:vporof][:vp]]

Comment on attachment 651170 [details] [diff] [review]
removed the extra spaces from debugger.dtd

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

All of this looks good to me. Just one minor change and we're good for landing.

::: browser/locales/en-US/chrome/browser/devtools/debugger.dtd
@@ +45,4 @@
>  
>  <!-- LOCALIZATION NOTE (debuggerUI.stepOutButton): This is the tooltip for the
>    -  button that steps out of a function call. -->
> +<!ENTITY debuggerUI.stepOutButton.tooltip    "Step Out (F8)">

The shortcut for step out is Shift+F8, as defined in debugger.xul. Please change this here to Step Out (Shift+F8). Don't worry about using U+21E7 ⇧ for now, we can do it in a follow up.

Comment 8

[Victor Porof [:vporof][:vp]]

Comment on attachment 651171 [details] [diff] [review]
edited debugger.properties

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

Yup, this is good.

Comment 9

[Abhishek Potnis [:avp]]

Attachment: edited debugger.dtd

Comment 10

[Victor Porof [:vporof][:vp]]

Rob, wanna review these so we can land them?

Comment 11

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 651171 [details] [diff] [review]
edited debugger.properties

 # LOCALIZATION NOTE (pauseLabel): The label that is displayed on the pause
 # button when the debugger is in a running state.
-pauseTooltip=Click to pause
+pauseTooltip=Click to pause (F6)

you need to change the property name to something new here so our localization tools can pick it up. Common practice is to append a number to the end of the name like:

pauseTooltip1=Click to pause (F6)

(I know, it's dumb. It's a convention we have to live with)

 # LOCALIZATION NOTE (resumeLabel): The label that is displayed on the pause
 # button when the debugger is in a paused state.
-resumeTooltip=Click to resume
+resumeTooltip=Click to resume (F6)

same here.

cancelling review. If you like, we can update the patch to deal with the above issues or you can do it yourself.

thanks!

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 654584 [details] [diff] [review]
edited debugger.dtd

   -  the button that steps over a function call. -->
-<!ENTITY debuggerUI.stepOverButton.tooltip   "Step Over">
+<!ENTITY debuggerUI.stepOverButton.tooltip   "Step Over (F7)">

same thing for dtd files. :/

l10n is hard.

Comment 13

[Victor Porof [:vporof][:vp]]

I'll fix these in bug 785889, since we're moving them around anyway.