1293048 - Change Ctrl+L keyboard shortcut for hyperlinks to Ctrl+K (standard/default for inserting links in Office applications)

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Macho Philipovich]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160728203720

Steps to reproduce:

Select some text in the compose window and press Ctrl+K


Actual results:

Nothing


Expected results:

Open the "link" dialog to add a hyperlink. This is a standard shortcut in many other applications (word processors, etc.) and does not currently seem to be attached to any other function in the compose window.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: ctrlK.patch

We used CTRL+L. MS Office and Libre Office use CTRL+K. So it makes sense to use the same shortcut.

Comment 2

[Richard Marti (:Paenglab)]

Jörg, sorry about the lot of white space changes but this files had a lot of them and my editor automatically removes trailing white spaces.

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

Hi Philip, the changes are mostly in /editor/ui, so they'd affect SM, too. What do you say to <ctrl>K to insert a link instead of <ctrl>L?

Does anyone else have an opinion?

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

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

Thanks for the patch. Before we go ahead we need to decide:
1) Do we want to go ahead with <ctrl>K in TB?
2) If so, does SM want it too, or can we do it so SM is not affected?
So right now I can't review it. See bug 1224471 where they want to use <ctrl>K for spellchecking (or they don't?). TB uses <ctrl><shift>P but in SM that's already used in other contexts.

Whether the questions below are relevant depends on SM's answer.

::: editor/ui/composer/content/editorOverlay.xul
@@ +107,5 @@
>           command="cmd_increaseFontStep"
>           modifiers="accel"/>
>  
>      <key id="insertlinkkb"
> +         key="&insertLinkCmd2.key;"

If we were to go ahead with this, could this change be moved to mail/components/compose/content/editorOverlay.xul so editor/ui/composer/content/editorOverlay.xul remains unchanged?

@@ +346,5 @@
>                  accesskey="&insertTableCmd.accesskey;"
>                  command="cmd_InsertTable"/>
>        <menuitem id="insertLink"
> +                label="&insertLinkCmd2.label;"
> +                accesskey="&insertLinkCmd2.accesskey;"

If we were to go ahead with this, isn't the change in mail/components/compose/content/editorOverlay.xul sufficient so SM is not affected?

::: editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
@@ -28,5 @@
>  <!ENTITY insertMenu.label "Insert">
>  <!ENTITY insertMenu.accesskey "I">
> -<!ENTITY insertLinkCmd.label "Link…">
> -<!ENTITY insertLinkCmd.accesskey "L">
> -<!ENTITY insertLinkCmd.key "L">

If we were to go ahead with this, could we leave insertLinkCmd in place so we don't affect SM?

Comment 5

[Richard Marti (:Paenglab)]

Attachment: ctrlK.patch for TB only

This patch is for TB only.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

Why don't we wait for SM's answer? Then we can decide which patch to use.

Comment 7

[Richard Marti (:Paenglab)]

I would say Ian answers for SM.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

Sure.

Comment 9

[Ian Neal]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

I am happy to switch to Ctrl-K for links in SM for the reasons given wrt *Office products.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 8778706 [details] [diff] [review]
ctrlK.patch

Looks right and works for me.
r=jorgk.
I don't think the "TB only" solution is required now.

Comment 11

[Richard Marti (:Paenglab)]

Comment on attachment 8778788 [details] [diff] [review]
ctrlK.patch for TB only

Okay, then I cancel the review for the TB only patch.

Comment 12

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/49753d8cf617
Bug fixed within two days of being raised, how is that for service?

Comment 13

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+2) from comment #12)
> https://hg.mozilla.org/comm-central/rev/49753d8cf617
> Bug fixed within two days of being raised, how is that for service?

That's AWESOME!

However, unfortunately, our users know nothing about this change :/
So they'll get stuck trying to use the old keyboard shortcut, and very soon we'll see bugs flooding...

Please ensure the following workflow for keyboard shortcut bugs:

1) Make the bug block
> Bug 713979 (alias: tb-keyboard-tracker).
You can actually type the alias "tb-keyboard-tracker" into the blocks field.

2) If we're changing/adding keyboard shortcuts, we need to update documentation and release notes.

a) open a separate bug for updating documentation, and make it block
> Bug 714031 (alias: tbkbd-doc-tracker).
If you're lazy, at least add the bug with the changes to tbkbd-doc-tracker.

b) CC people from support on the documentation-update bug, so that their progress can be tracked as they report back. Ask them to update the central TB keyboards shortcut documentation, or even better, update it yourself:
https://support.mozilla.org/t5/Tips-and-Tricks/Keyboard-Shortcuts-TB/ta-p/13987
OT, that page is an a deplorable state after migration to the new support system (e.g. many versioned shortcuts now showing as duplicates). Who is responsible for that!?

c) add "relnote" keyword to the bug with the changes, or the documentation updating bug.

d) Ensure that "What's new?" Section of release notes covers any keyboard shortcut changes/additions in the "Changed"/"New" section. Who can edit that document?
> https://www.mozilla.org/en-US/thunderbird/52.0/releasenotes/

Breaking existing keyboard shortcuts is sometimes a necessary evil; however, to avoid user frustration and user errors, we really owe it to our users to inform them about such changes as soon as they occur. Any changes of keyboard shortcuts definitely belong into release notes.

Wrt this bug, there's absolutely no documentation whatsoever; TB52 flavor of keyboard shortcuts page does not even exist, and there's no entry in "What's new" section of release notes. Jörg, could you update that?

Comment 14

[Thomas D. (:thomas8)]

For better visibility, here the link to TB keyboard documentation again:
> https://support.mozilla.org/t5/Tips-and-Tricks/Keyboard-Shortcuts-TB/ta-p/13987

Comment 15

[Thomas D. (:thomas8)]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #13)
> (In reply to Jorg K (GMT+2) from comment #12)
> > https://hg.mozilla.org/comm-central/rev/49753d8cf617
> > Bug fixed within two days of being raised, how is that for service?
> 
> That's AWESOME!
> 
> However, unfortunately, our users know nothing about this change :/

I spoke too soon... In this case, there's in-product documentation in the menus (caveat: not applicable for all keyboard shortcuts!). Still, this should be mentioned in release notes (probably also spelling out the reason, so that we don't get blamed for random changes), and central documentation must be updated. Also, adding dependencies to respective tracker bugs on BMO.

Comment 16

[Thomas D. (:thomas8)]

Sorry for bugspam...
Fwiw, I've changed the summary not because I like it long and complex, but to include relevant keywords so as to improve retrievability and enable users to find this in BMO. As a rule of thumb, the full EN menu path (Insert > Link) or respective keywords ("inserting links...") should normally be included, so that searches like "keyboard shortcut Insert Link" can succeed. Thanks.

Comment 17

[Jorg K (CEST = GMT+2)]

We change keyboard shortcuts all the time, for example bug 1335218, bug 1308776 (your own bug), bug 1320405 or bug 1328167 (for a few I can remember). Nothing ever makes it into any documentation.

You're right, we already had a complaint: bug 1355313.

Comment 18

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+2) from comment #17)
> We change keyboard shortcuts all the time, for example bug 1335218, bug
> 1308776 (your own bug), bug 1320405 or bug 1328167 (for a few I can
> remember).

Hold on, please don't confuse 'Keyboard Shortcuts' with 'Access Keys'. Most of the above bugs, including "my" bug, have only changed/added access keys. Only bug 1328167 (currently NEW) is a keyboard shortcut bug.
Changing access keys only applies to one specific localized version of TB (mostly EN), so that's much less significant and we can't easily relnote that because we don't have language-specific relnotes.

> Nothing ever makes it into any documentation.

How can we change that? Perhaps along my workflow proposals of comment 13?

> You're right, we already had a complaint: bug 1355313.

We'll certainly see more, this is a highly exposed feature.
We could consider keeping Ctrl+L as an alternative, but that's wasting a shortcut which might bite us lateron.

In the long run, I have some doubts if Ctrl+K for hyperlink is sustainable when we'll have "compose-in-a-tab" and there might well be a global search box on the top toolbar, for which we currently use Ctrl+K, consistent with FF. But alas, by the time we get compose-in-a-tab, we'll probably use voice control instead of keyboard shortcuts...

Comment 19

[Jorg K (CEST = GMT+2)]

I've added an entry in the TB 52.0 release notes now. Sadly they are superseded by 52.0.1, so I doubt anyone will ever read them. You need to wait a while before the page refreshes.