fix: don't append Shift modifier text twice to accelerators (backport: 4-0-x)

[brenca]

Description of Change

Backport of #15400 to 4-0-x

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• PR title follows semantic commit guidelines

Release Notes

Notes: fixed some accelerators having Shift appended to them twice

[MarshallOfSound]

@brenca Can you manually confirm what the Control+Shift+= accelerator looks like and what the Control++ accelerator looks like after this PR?

[brenca]

@MarshallOfSound
Before:

After:

const template = [
    {
      label: "Menu item",
      submenu: [
        {
          label: "One",
          id: "one",
          type: "normal",
          accelerator: "Shift+Backspace",
          click: () => {
            console.log('asd');
          }
        },
        {
          label: "Two",
          id: "two",
          type: "normal",
          accelerator: "Control+Shift+=",
          click: () => {
            console.log('asd');
          }
        },
        {
          label: "Three",
          id: "three",
          type: "normal",
          accelerator: "Control++",
          click: () => {
            console.log('asd');
          }
        }
      ]
    }
  ];

[brenca]

@MarshallOfSound if I use Control+Plus it shows up as Control+Shift+= in all versions

[MarshallOfSound]

The usual suspects appear to still work 👍 Nice

[nornagon]

Would it be possible to encode @MarshallOfSound's intuition about what might have broken into a test? 🤞

[brenca]

@nornagon AFAIK what @MarshallOfSound asked about is how it gets displayed, which is not really testable from JS, but I might be wrong on that.

[nornagon]

Could you potentially expose the result of Accelerator::GetShortcutText to JS and test it that way?

[brenca]

@nornagon Yeah, that could work, gonna add that tomorrow to the PR for master

[nornagon]

where are these \u0000 coming from? that doesn't seem right...

[brenca]

That's what CI returned when I ran a pass to determine the values for Mac.

[nornagon]

that's where you got them from, but not where they came from :P

[brenca]

@nornagon my best guess would be that 0 at the end that we (apparently) keep during conversion from base::string16

[nornagon]

I think you could fix this by converting to a std::string before returning the value? It's surprising to me that that wouldn't be done automatically by the conversion into JS though.

[release-clerk]

Release Notes Persisted

fixed some accelerators having Shift appended to them twice