Closed Bug 1473438 Opened 6 years ago Closed 5 years ago

Tooltip for the DOM and Style Inspector incorrectly shows its keyboard shortcut as "(Ctrl+Shift+)C"


(DevTools :: Inspector, defect, P3)

62 Branch


(firefox-esr60 unaffected, firefox62 wontfix, firefox64 wontfix)

Tracking Status
firefox-esr60 --- unaffected
firefox62 --- wontfix
firefox64 --- wontfix


(Reporter: scook0+bugzilla, Unassigned)



(Keywords: regression)


(1 file)

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

Steps to reproduce:

1. Press F12 to open the developer tools.
2. Hover over the "Inspector" tab to view its tooltip.

Actual results:

The tooltip displays as "DOM and Style Inspector (Ctrl+Shift+)C".

Note that the "C" is outside the (parentheses).

Expected results:

The tooltip should be "DOM and Style Inspector (Ctrl+Shift+C)".

The "C" should be inside the (parentheses), together with its modifiers.
This seems to have been accidentally introduced by the changes for bug 1014090.

Presumably the concatenation of "Ctrl+Shift+" with l10n("inspector.commandkey") should take place inside the second argument position of the first l10n call.
I imagine this only affects non-Mac platforms.
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
Blocks: 1014090
Severity: normal → trivial
Has STR: --- → yes
Component: Untriaged → Inspector
Ever confirmed: true
Flags: needinfo?(pbrosset)
Keywords: regression
Product: Firefox → DevTools
Hi there,

From what I can tell, this was added during when I was working on Bug 1014090 a few weeks back - oops! Here's the link to the bug:

I think this patch I've attached should resolve it, but I don't have a non OS X machine handy to test this on - sorry about that.

I'm at one of the Mozilla nights, so I'll see if someone else is able to test this, and if so I'll update some more.

Patrick, I've requested you looking over it as you mentioned in slack earlier today, and you were mentoring me when we first looked over this part before - hope that's okay.
Attachment #8990075 - Flags: review?(pbrosset)
Assignee: nobody → chris
Thanks very much Chris for taking a look at this. I will review the patch and test it locally today.
Flags: needinfo?(pbrosset)
Comment on attachment 8990075 [details] [diff] [review]
Put the Ctrl+Shift tooltip back inside the parentheses

Review of attachment 8990075 [details] [diff] [review]:

Looks good to me, I've tested locally on Mac and Windows and it works fine on both.
I've made a comment below about changing a variable name, minor stuff, but would be great to fix.
Also, I have the impression that you might have based this patch on an older revision of the mozilla source code, because importing your patch locally did not work well for me. I had to merge manually.
Can you make sure you pull the latest, and then redo your changes on top of that?

::: devtools/client/definitions.js
@@ +79,3 @@
>        return l10n("inspector.mac.tooltip", cmdShiftC, cmdOptC);
>      } else {
> +      const cmdShiftC = "Ctrl+Shift+" + l10n("inspector.commandkey");

This variable name should be ctrlShiftC instead of cmdShiftC to be totally correct.
Attachment #8990075 - Flags: review?(pbrosset) → feedback+
Priority: -- → P3

Chris, as you still interested in fixing this? If so, please see comment 6.

Unassigning this bug as it hasn't been touched in 8 months and Chris hasn't responded to the needinfo 2 months ago.
Please feel free to pick it up again if you did intend to work on it still.

Assignee: chris → nobody
Flags: needinfo?(chris)

I recently fixed this issue in a duplicate: bug 1528608
I'll mark this as duplicate and close it.

Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.