[Affected versions]:

• Firefox 66.0a1 (20190114215201)

[Affected platforms]:

• Win 7 64-bit
• Mac OS X 10.13.3

[Steps to reproduce]:

1. Install https://addons.mozilla.org/en-US/firefox/addon/shortkeys/
2. Navigate to Extensions from about:addons page.
3. Click on the “Keyboard” button.
4. The detailed page with the shortcuts that can be assigned to the extension is displayed.
5. Assign a shortcut key to a command, for example “Alt+L”.
6. Assign the same shortcut key to another command.

[Expected results]:

• The user is not allowed to have duplicated shortcut key to multiple commands.

[Actual results]:

• The user can assign the same shortcut key to multiple commands.
• Having the same shortcut key assigned to multiple commands is confusing.

Please see the attached screenshot.

It would be nice to handle this, but I don't think it's a blocker. Extensions can already choose the same shortcut, so perhaps allowing it but showing a warning message would be enough.

Emanuela, do you have any thoughts on how we should handle commands with the same shortcut? Multiple extensions (or even the same extension) can define the same shortcut and I don't think there's a clearly defined command that will be executed in that case, but only one of them will be.

(In reply to Mark Striemer [:mstriemer] from comment #1)

It would be nice to handle this, but I don't think it's a blocker. Extensions can already choose the same shortcut, so perhaps allowing it but showing a warning message would be enough.

Emanuela, do you have any thoughts on how we should handle commands with the same shortcut? Multiple extensions (or even the same extension) can define the same shortcut and I don't think there's a clearly defined command that will be executed in that case, but only one of them will be.

Ideally, it would be nice not to let this situation happening, but as Mark wrote above, I don't think it's possible.

I'm struggling with the warning message since I don't know which information we can detect.

So far I have in mind something like:

One or more extensions are using {insert shortcut} as a shortcut. It may cause unexpected behavior.

• The conflict should not block the user to continue to use Firefox.
• The message disappears when the user resolves the conflict.
• The message should only appear in the shortcut tab.

Needinfo Meridel to get her opinion on this issue.

Proposed copy for multiple extensions using the same shortcut AND for one extension using the same shortcut multiple times:

Copy Template:

[Command name] is being used as a shortcut for:

[Extension A Name]: [Shortcut function A]
[Extension A Name]: [Shortcut function B]
[Extension B Name]: [Shortcut function C]

Duplicate shortcuts may cause unexpected behavior.

Example:

Ctrl+W is being used as a shortcut for:

• uBlock Origin: Open zapper
• uBlock Origin: Open element picker
• Tomato Clock: Start a new tomato timer

Duplicate shortcuts may cause unexpected behavior.

Updated string based on design proposal.

Template:

[Command]is being used as a shortcut for: [Extension A Name]-[Shortcut function A], [Extension A Name]-[Shortcut function B], [Extension B Name]-[Shortcut function B]. Duplicate shortcuts may cause unexpected behavior.

Example:

Ctrl+W is being used as a shortcut for: uBlock Origin - Open zapper, uBlock Origin - Open element picker, Tomato Clock - Start a new tomato timer. Duplicate shortcuts may cause unexpected behavior.

We're going to want to avoid the comma separated list, and an unordered list will make this quite large.

Instead, the message should be more generic:

Ctrl+W is being used as a shortcut in more than one case. Duplicate shortcuts may cause unexpected behavior.

To help users identify duplicates, the input should have a warning style when it has a duplicate [1].

When the input is focused the error message should initially be set to "Duplicate shortcut", but with warning styles instead. These colours should match a message bar, background of yellow 50, text of yellow 90.

NI? from emanuela to confirm

[1] https://design.firefox.com/photon/components/input-fields.html#behaviors

(In reply to Mark Striemer [:mstriemer] from comment #5)

We're going to want to avoid the comma separated list, and an unordered list will make this quite large.

Instead, the message should be more generic:

Ctrl+W is being used as a shortcut in more than one case. Duplicate shortcuts may cause unexpected behavior.

To help users identify duplicates, the input should have a warning style when it has a duplicate [1].

When the input is focused the error message should initially be set to "Duplicate shortcut", but with warning styles instead. These colours should match a message bar, background of yellow 50, text of yellow 90.

NI? from emanuela to confirm

[1] https://design.firefox.com/photon/components/input-fields.html#behaviors

Yes, that's all correct.
Thank you, Mark.

Hi Mark, can I take this one too?

Absolutely! I think comment 5 lists all the changes we'd like here.

Hi Mark,
from what I understood attached is the required behavior, please let me know if this is fine or I missed something.
PS: I am not sure if I got the display of message proper(Ctrl+W is being used as a shortcut in more than one case. Duplicate shortcuts may cause unexpected behavior)

Hey, sorry for the delay.

I think we want to use the message-bar styles for this, and it should probably go at the top of the page. This could cause some unwanted shifting when the message is hidden, but hopefully that isn't too jarring.

When the input is focused (imagine this input has the duplicate warning) then the error message should appear with the yellow warning styles on focus including the "Duplicate shortcut" message.

Hi Mark,
No worries, I got it now, I will work on the fix.

Hi Mark,
I have implemented this, but the jump is very visible.
Shall I proceed with this and write tests?

Post discussion with Mark following is the behavior we intend to ship.

1. Show individual warning message bar at top for each duplicate shortcut(persistent)
2. every input with duplicate shortcut should have warning focus style (as in #c9)
3. When input with duplicate shortcut is focused warning message "Duplicate shortcut" should appear (as in #c12)
4. When user resolves conflicting shortcuts, the warnings(message bar and warning-focus) will disappear respectively.

Notify users when there is a duplicate shortcut

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5974ae084211
Notify users when there is a duplicate shortcut r=mstriemer,fluent-reviewers,flod,Gijs

Backed out changeset 5974ae084211 for causing failures in browser_manage_shortcuts_hidden.js/browser_shortcuts_duplicate_check.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/c1683d4d191a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&tochange=c1683d4d191a40572b799947933b65aa94384209&fromchange=5974ae0842115acc3819232a089825997da3c53f&selectedJob=261746705

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=261746705&repo=autoland&lineNumber=13362

[task 2019-08-15T08:48:59.349Z] 08:48:59 INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_interaction_telemetry.js | A promise chain failed to handle a rejection: extension is null - stack: buildDuplicateShortcutsMap/<@chrome://mozapps/content/extensions/shortcuts.js:295:11

[task 2019-08-15T08:50:29.423Z] 08:50:29 INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js | A promise chain failed to handle a rejection: extension is null - stack: buildDuplicateShortcutsMap/<@chrome://mozapps/content/extensions/shortcuts.js:295:11

[task 2019-08-15T08:51:59.755Z] 08:51:59 INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts_hidden.js | A promise chain failed to handle a rejection: extension is null - stack: buildDuplicateShortcutsMap/<@chrome://mozapps/content/extensions/shortcuts.js:295:11

[task 2019-08-15T08:54:17.808Z] 08:54:17 INFO - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_shortcuts_duplicate_check.js | A promise chain failed to handle a rejection: extension is null - stack: buildDuplicateShortcutsMap/<@chrome://mozapps/content/extensions/shortcuts.js:295:11

Failures seem to be caused by shortcuts.js and about:addons

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f69698a5cc46
Notify users when there is a duplicate shortcut r=mstriemer,fluent-reviewers,flod,Gijs

https://hg.mozilla.org/mozilla-central/rev/f69698a5cc46

Hello,
Checked on Firefox 70.0a1 (Build ID:20190820215247) on Windows 10 64-bit and Mac 10.13.6.
The same shortcut cannot be stored on the same or multiple extensions because the text is erased after the keys are released.
Further clarification is needed on the following:

1. Since the duplicate shortcut is not persistent, no "individual warning message bar at top" is shown. Is that the expected behavior?
2. The color of the warning is red, not yellow, as requested in comment 10 - which is the correct behavior?
3. When input with duplicate shortcut is focused the "Already in use by [extension name]" warning is displayed. The warning message is different from last established in comment 13 - can you please confirm this is intended?
4. Also in accordance with comment 13 " When user resolves conflicting shortcuts, the warnings(message bar and warning-focus) will disappear respectively." Should the warning still be displayed, after the shortcut is erased, until the user sets the focus somewhere else in the page?
   Please see the "Duplicate shortcut in other extension" attachment.
   Thanks

Hallo,

This bug is for specific case where user installs two addons with same shortcuts(preset by add-on developers not by user).
Please find replies inline.

(In reply to Miruna Curtean from comment #20)

Hello,
Checked on Firefox 70.0a1 (Build ID:20190820215247) on Windows 10 64-bit and Mac 10.13.6.
The same shortcut cannot be stored on the same or multiple extensions because the text is erased after the keys are released.

To reproduce this case, you need to install add-ons with same shortcut keys, user can not assign already used shortcut.

Further clarification is needed on the following:

1. Since the duplicate shortcut is not persistent, no "individual warning message bar at top" is shown. Is that the expected behavior?

Already assigned shortcuts cannot be reassigned so when shortcut is erased, that means shortcut is not saved. When focus is lost it retains its previous value.

2. The color of the warning is red, not yellow, as requested in comment 10 - which is the correct behavior?

Yes the yellow warning color should be only for shortcuts which are used more than once.

3. When input with duplicate shortcut is focused the "Already in use by [extension name]" warning is displayed. The warning message is different from last established in comment 13 - can you please confirm this is intended?

The red one is more of error message, which prevents user to assign already assigned shortcut to another add-on. This is intended. User should in no way be able to assign same shortcut to multiple add-ons.

4. Also in accordance with comment 13 " When user resolves conflicting shortcuts, the warnings(message bar and warning-focus) will disappear respectively." Should the warning still be displayed, after the shortcut is erased, until the user sets the focus somewhere else in the page?

Once the shortcut is set(basically a non conflicting one) the warning message bar will be gone.

Installing following add-ons will generate the case:

https://addons.mozilla.org/en-US/firefox/addon/bookmark-shortcuts
https://addons.mozilla.org/en-US/firefox/addon/easy-container-shortcuts

Verified this fix on Firefox 70.0a1 with Build ID: 20190821215524 on Windows 10 64-bit and Mac 10.13.6.
On Windows I've had to install an additional addon in order to fulfill the test conditions.
https://addons.mozilla.org/en-US/firefox/addon/webext-private-bookmarks
On Windows, where there are only two duplicate shortcuts, all the behaviors appear as expected:
Individual, persistent warnings appear at the top of the page until the user resolves the conflict. The "Duplicate shortcut" message appears at input on the duplicate shortcut and the warning focus style is also displayed from each duplicate shortcut.

I've noticed on MAC, by installing the "Bookmarks Shortcuts" and "Easy Container Shortcut" addons, that while there are 9 duplicate shortcuts, a maximum of 5 persistent, individual warning message bars will show at the top of the page. Once one of them is resolved, another will be added for the remaining conflict.

The only remaining concern is for these cases in which there are more than 5 duplicate shortcuts is that the warning message bars are shown in reverse order ( starting with ALT+SHIFT+9) and the user needs to expand the shortcut list to resolve them in order.
Will this be resolved?

We restrict max messages to 5, for duplicate shortcuts which are hidden we highlight warning on show more button.
I am not sure about the order though.

Should be able to reverse the order they appear with the message bar's reverse attribute. Can you file a follow up?

Added https://bugzilla.mozilla.org/show_bug.cgi?id=1577485 to deal with the warnings reverse order.
On another look I've also noticed that in the case of the SHIFT+ALT+ shortcuts, the keys are not displayed in the correct order either and created ticket https://bugzilla.mozilla.org/show_bug.cgi?id=1577474 .
The intended behavior works as expected in comment 13 as verified once more on Windows 10 64-bit/ Ubuntu 18.04.2 LTS x 64 bit on Firefox 70.0a1 20190828214452 and on Mac 10.13.6 on Firefox 70.0a1 20190822215453.