Ctrl-Shift-K bound to 2 functions in Compose

Bug #68456 reported by C Snover on 2006-10-26
8
Affects Status Importance Assigned to Milestone
Mozilla Thunderbird
Fix Released
Medium
thunderbird (Ubuntu)
Low
Mozilla Bugs

Bug Description

Ctrl-Shift-K is bound to both "Format -> Discontinue link" as well as to "Options -> Check Spelling".

Version 1.5.0.7 (20060922)
mozilla-thunderbird 1.5.0.7-0ubuntu0.6.06
thunderbird-locale-en-gb 1.5.0.2ubuntu1-3

Jeremy Teale (jteale) wrote :

Just confirming for 1.5.0.7-0ubuntu1 on Edgy.

Changed in mozilla-thunderbird:
status: Unconfirmed → Confirmed
Changed in thunderbird:
status: Unknown → In Progress
Changed in mozilla-thunderbird:
importance: Undecided → Low

Assigned to Mozilla Team

Changed in mozilla-thunderbird:
assignee: nobody → mozilla-bugs
Ara Pulido (ara) wrote :

I can confirm that this is happening in Intrepid:

thunderbird:
  Installed: 2.0.0.16+nobinonly-0ubuntu2
  Candidate: 2.0.0.16+nobinonly-0ubuntu2
  Version table:
 *** 2.0.0.16+nobinonly-0ubuntu2 0
        500 http://es.archive.ubuntu.com intrepid/main Packages
        100 /var/lib/dpkg/status

Changed in thunderbird:
status: Confirmed → Triaged

User-Agent: Mozilla/5.0 (X11; U; Linux i686; cs-CZ; rv:1.9.0.2) Gecko/2008100707 Fedora/3.0.2-1.fc10 Firefox/3.0.2
Build Identifier: thunderbird-2.0.0.17-1.fc10.x86_64

(originally filed as https://bugzilla.redhat.com/show_bug.cgi?id=468130)

In composer or html-mail-composer, the Format>Discontinue Link menu item
advertises Control-Shift-k as a valid hotkey, however pressing this key brings
up the "Check Spelling" window instead.

Reproducible: Always

Steps to Reproduce:
1.look at Format/Discontinue Link and Options/Spellchecking Options in HTML Composer
2.
3.
Actual Results:
same key shortcut

Expected Results:
shouldn't be

Bryan, I wonder if unlink shouldn't be ctrl+shift+L instead of K, because _link_ is ctrl+K, under Insert. I don't know the history of why these don't match up.

Regardless, ctrl+shift+K is WFM, does not invoke spell using trunk.
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081104 Shredder/3.0b1pre

It keeps checking spelling on Mac. Confirming bug.

Created an attachment (id=347226)
patch v1

This seems to fix the issue by switching the access key for discontinue link to cmd-shift-l though I'm having reservations about my approach.

Check spelling is Ctrl+K, not Ctrl+Shift+K. (Is that not the case on mac?)

Re comment 1, I imagine it's not "L" since you don't remove the link, just end it.

Magnus, have you gone to the dark side? http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/messengercompose.xul#232 and my builds say it's accel-shift-k on Linux and Mac, and only accel-k (or F7) on Windows.

Only momentarily. Back on the light side now;)

I was wrong about Ctrl+Shift+L too, looking more closely I think it's currently not "discontinue link"/"remove links" - which are the same menu - because Ctrl+Shift+L is Open Web Location in seamonkey composer... and then we just copied it.

I'm not sure why it's different on different platforms now.
CVS blame only says
  1.56 <email address hidden> 2004-02-04 21:10
  spell check on unix should be ctrl-shift-k like seamonkey.

  Thanks to Stephen Walker for the patch

Might be because of the old gtk limitation, which isn't an issue on trunk - see bug 311756 comment 35. I don't see a reason we can't use the same combination for all platforms now.

And at least it would be very logical to use ctrl-shift-L for that, to match insert link (ctrl-L).

we're going to be using ctrl-k for the thunderbar (bug 452281) so I'd recommend we go with a ctrl-L combination as magnus suggests in comment 6

(From update of attachment 347226)
Minusing, let's make it the same for all platforms, and preferably "L" as per previous discussion.

I'm not sure it actually affects thunderbar though as this is only the composition window hotkeys.

Created an attachment (id=347801)
patch v1.1

I'm suspecting I'm missing something else, do I need to change the accelerators or something to keep them identical cross-platform?

Not quite ready for review until this is sorted out.

(From update of attachment 347801)
wrong wrong..

Created an attachment (id=347803)
patch v1.2

This should taste better. :)

Access key for removing links changed to L, and ifdef removed for the case of checking spelling, standardizing on (ctrl or cmd)-shift-l.

(From update of attachment 347803)
Wrong editorOverlay.xul, and I think in these cases you should bump the entity so localizers can keep up.

(Unless seamonkey wants the same change... but that would need more work.)

> I'm not sure it actually affects thunderbar though as this is only the
> composition window hotkeys.

True in the current separate window it's not likely to affect it. I guess I meant to say I wanted to keep it open for a future where we might also have the compose area inside a tab.

(In reply to comment #12)
> (From update of attachment 347803 [details])
> Wrong editorOverlay.xul, and I think in these cases you should bump the entity
> so localizers can keep up.

So |formatRemoveLinks.key| should be changed to |formatRemoveLinks.key2| ? (then the former will be changed throughout)

Another thing is, in both
http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editorOverlay.xul#101
and
http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/editorOverlay.xul#69

the modifier keys are already "accel, shift" already, so I suppose they don't need to be changed?

Created an attachment (id=348137)
patch v1.3

Trying again. Entity bumped. formatRemoveLinks.key isn't referenced anywhere else with modifier keys of "accel" only -- existing ones are already "accel,shift".

I didn't find any relevant ifdefs to be removed -- am I missing something somewhere?

(In reply to comment #14)
> So |formatRemoveLinks.key| should be changed to |formatRemoveLinks.key2| ?
> (then the former will be changed throughout)

The tools localizers use can do some automatic stuff for certain entity name endings, so it should still end in '.key'. Something like formatRemoveDiscontinueLinks.key

> the modifier keys are already "accel, shift" already, so I suppose they don't
> need to be changed?

No, but remove the platform ifdef for the spelling one.

Also, this patch would give seamonkey composer two functions for ctrl+shift+L, no?

Created an attachment (id=348312)
patch v1.4

Incorporates all of Magnus' comments so far, but without the SM "conflicting-shortcut" fix.

What should the conflicting SM shortcuts be changed to then? or should the SM fix be in a separate bug?

Well, seems at least Ctrl+K should be unused, so perhaps that?
It should all happen in the same patch. Note that you'll have to get neil's reviews as well.

Created an attachment (id=348585)
now with SM fixes

Now shortcut for openRemoteCmd.key is ctrl-k as suggested by Magnus. Bumped entity to openRemoteCmd2.key, TB changes remain identical to v1.4.

Requesting review from Neil for this SM change first before I re-request for the TB changes.

(From update of attachment 348585)
Ctrl+K doesn't make sense for two reasons:
1. The same reason that we didn't use it for spellcheck in the first place: some Linux keyboard layouts map it to delete to end of line.
2. The shortcut for open web location in SeaMonkey's browser is also Ctrl+Shift+L.

I'm still not used to the new order of the flags in the dropdown :-(

(In reply to comment #21)
> (From update of attachment 348585 [details])
> Ctrl+K doesn't make sense for two reasons:
> 1. The same reason that we didn't use it for spellcheck in the first place:
> some Linux keyboard layouts map it to delete to end of line.
> 2. The shortcut for open web location in SeaMonkey's browser is also
> Ctrl+Shift+L.

If Ctrl+K isn't suitable, what should be the correct mappings?

Now there's Ctrl+Shift+L being currently used in 3 places, open web location, open remote cmd, discontinue link.

cc KaiRo as I don't know who does UI for SM.

Neil is our official "UI tsar" for SeaMonkey, and he already commented here :)

(In reply to comment #23)
> If Ctrl+K isn't suitable, what should be the correct mappings?
How about Ctrl+Shift+C for check spelling?

> Now there's Ctrl+Shift+L being currently used in 3 places, open web location,
> open remote cmd, discontinue link.
I don't think "currently" applies to discontinue link, that's still only a suggestion (from as far back as comment #1); open remote cmd is basically editor's version of open web location.

Ctrl+Shift+C = mark all read in thread pane, which is kind of destructive - bad enough it's close to ctrl+c(copy).

If there is no other alternative I'd sooner see Ctrl+Shift+L used in 3 places for 3 different reasons.

(In reply to comment #25)
> (In reply to comment #23)
> > If Ctrl+K isn't suitable, what should be the correct mappings?
> How about Ctrl+Shift+C for check spelling?

Sigh~ sure this is ok from what Wayne said in comment #26?

>
> > Now there's Ctrl+Shift+L being currently used in 3 places, open web location,
> > open remote cmd, discontinue link.
> I don't think "currently" applies to discontinue link, that's still only a
> suggestion (from as far back as comment #1); open remote cmd is basically
> editor's version of open web location.

Yes, my bad, I should have said s/currently/currently suggested to be

Ugh. So, tb could of course use Ctrl+Shift+L anyways... but I suppose seamonkey needs a fix too. (No hotkey for it, or then perhaps the suggested Ctrl+Shift+C?)

(In reply to comment #28)
> Ugh. So, tb could of course use Ctrl+Shift+L anyways... but I suppose seamonkey
> needs a fix too. (No hotkey for it, or then perhaps the suggested
> Ctrl+Shift+C?)
Sadly the key is in editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd

Neil, Magnus, I'd like to push forward on this, should there be a consensus on the hotkeys for TB/SM?

Basically patch 1.4 shifts the hotkey for DiscontinueLink from Ctrl-Shift-K to Ctrl-Shift-L, but the concept cannot be applied as-is to SM since Ctrl-Shift-L is used for other functions in SM.

If I recall my IRC logs correctly in a conversation with Neil, here's the current SM situation:

NeilAway> nth10sd: so, the free non-shifted keys seems to be d, e, h, j and r, while the used shifted keys seem to be a, g, k, l, n, s, v, y

How about Ctrl+Shift+I for discontinue link?

Sorry, but that's DOM Inspector's hot key.

Huh? Ctrl+Shift+I doesn't seem to open DOMi in either composer or mail compose window. (And it's not listed to do it in the menus either.)

I suggested "I" to get at char that's in the "Link" word, to match the similar menu hot keys around it.

Doesn't work for me.

Anyway, change the Check Spelling... to Ctrl+Shift+P then?

Yeah, that's fine by me.

(In reply to comment #36)
> Doesn't work for me.
>
> Anyway, change the Check Spelling... to Ctrl+Shift+P then?

So should I abandon the patch that changes mainly Format/Discontinue Link and change Options/Check Spelling to Ctrl+Shift+P instead?

Created an attachment (id=356532)
patch that changes Options/Check Spelling to Ctrl-Shift-P instead

Created an attachment (id=356535)
oops, prev patch didn't bump an entity

(From update of attachment 356535)
> <key id="checkspellingkb"
>- key="&checkSpellingCmd.key;"
>+ key="&checkSpellingCmd.key3;"
> observes="cmd_spelling"
> modifiers="&accel.emacs_conflict;"
Change this to "accel,shift" please.

> <!ENTITY checkSpellingCmd.label "Check Spelling">
> <!ENTITY checkSpellingCmd.accesskey "h">
>-<!ENTITY checkSpellingCmd.key "K">
>+<!ENTITY checkSpellingCmd.key3 "P">
Nit: "p" to match "Spelling"

> <key id="key_checkspelling"
>- key="&checkSpellingCmd.key;"
>+ key="&checkSpellingCmd.key3;"
> command="cmd_spelling"
> modifiers="&accel.emacs_conflict;"
Ditto please.

> <!ENTITY checkSpellingCmd.label "Check Spelling…">
>-<!ENTITY checkSpellingCmd.key "K">
>+<!ENTITY checkSpellingCmd.key3 "P">
Ditto please.

Created an attachment (id=356546)
patch 2.2

Patch 2.2 that incorporates Neil's comments, bringing over sr+, requesting review from mkmelin.

(From update of attachment 356546)
Looks good, Gary!

From a localizers point of view, please don't rename "xxxx.key" to "xxxx.key3". Please rename the key again to "xxxx3.key".

(In reply to comment #47)
> From a localizers point of view, please don't rename "xxxx.key" to "xxxx.key3".
> Please rename the key again to "xxxx3.key".

Mhh, maybe this wouldn't be a good idea, too. "xxxx3.key" wouldn't match the labels name. So rename the label and the key?

(In reply to comment #42)
>(From update of attachment 356535)
>> <key id="checkspellingkb"
>>- key="&checkSpellingCmd.key;"
>>+ key="&checkSpellingCmd.key3;"
>> observes="cmd_spelling"
>> modifiers="&accel.emacs_conflict;"
>Change this to "accel,shift" please.
Bah, I thought the quotes would make it clear that I wanted the entire attribute changed...

(In reply to comment #48)
>(In reply to comment #47)
>>From a localizers point of view, please don't rename "xxxx.key" to "xxxx.key3".
>>Please rename the key again to "xxxx3.key".
>Mhh, maybe this wouldn't be a good idea, too. "xxxx3.key" wouldn't match the
>labels name. So rename the label and the key?
Yes, that makes sense, we'll call it checkSpellingCmd2.key etc.

Created an attachment (id=357145)
add on patch

Sorry about the "accel,shift" error, I mis-interpreted it.

Created an attachment (id=357147)
add on patch v2

Incorporates KaiRo's suggestions off IRC, fixes my |modifiers="accel,shift"| error.

Any chance we can get this checked in before the next suite nightlies?

(From update of attachment 357147)
r=mkmelin. I'll check this in in a moment.

*** Bug 236390 has been marked as a duplicate of this bug. ***

Changed in thunderbird:
status: In Progress → Invalid

*** Bug 475973 has been marked as a duplicate of this bug. ***

*** Bug 441331 has been marked as a duplicate of this bug. ***

Micah Gersten (micahg) wrote :

Upstream was resolved as a duplicate.

Changed in thunderbird:
status: Invalid → Unknown
milestone: none → 3.0
Changed in thunderbird:
status: Unknown → Fix Released
Launchpad Janitor (janitor) wrote :
Download full text (6.7 KiB)

This bug was fixed in the package thunderbird - 3.0+nobinonly-0ubuntu1

---------------
thunderbird (3.0+nobinonly-0ubuntu1) lucid; urgency=low

  * New Upstream Release 3.0 (THUNDERBIRD_3_0_RELEASE)
    - LP: #50902 - Thunderbird displays useless dialog
    - LP: #52667 - Thunderbird doesn't support RFC-2369
    - LP: #49033 - Doesn't recognize upper case extension (.JPG)
    - LP: #56465 - Per folder column widths
    - LP: #68456 - CTRL-Shift-K bound to 2 functions
    - LP: #79337 - Typo in Server Information for Add Account Wizard
    - LP: #1084 - No scroll on full headers list
    - LP: #62071 - Middle click on scrollbar pastes instead of jumping
    - LP: #119358 - Weak default authentication mode
    - LP: #120672 - No option to empty junk folder with right click
    - LP: #96566 - movemail doesn't work with default privs
    - LP: #122529 - Non-Thunderbird IMAP folders not visible to Thunderbird
    - LP: #241276 - Not able to paste image into thunderbird compose window
    - LP: #244635 - scrollboxes scroll to offset 0 when resized
    - LP: #259387 - "Edit Message as New" broken for eml messages
    - LP: #120281 - Editing a message from the drafts folder leaves line breaks
    - LP: #115484 - Dialogue boxes too large for 1024x768 resolution
    - LP: #320034 - Mail with self referencing headers breaks threading
    - LP: #160794 - shortcuts different in windows and linux
    - LP: #280987 - thunderbird keeps asking a password when working off-line
    - LP: #369150 - Thunderbird splits email addresses with non-ascii characters
                    and a comma in From: field
    - LP: #135066 - Thunderbird doesn't use Ubuntu icon theme
    - LP: #297301 - after authentication error the password is forgotten
    - LP: #487541 - thunderbird-bin crashed with SIGSEGV (AFS filesystem)
    - LP: #485224 - Thunderbird saves double attachment file name endings on
                    FAT32 and NTFS
    - LP: #482496 - When using SCIM ANTHY, autosaving fails, and then get asked
                    about sending in UTF-8

  [ Fabien Tassin <email address hidden> ]
  * Add build-depends on autoconf2.13, autotolls-dev, mozilla-devscripts
    libglib2.0-dev (>= 2.12), libstartup-notification0-dev, libbz2-dev,
    libpixman-1-dev, libdbus-1-dev (>= 1.0.0), libdbus-glib-1-dev (>= 0.60),
    libhal-dev (>= 0.5.8), libasound2-dev, libreadline5-dev | libreadline-dev,
    libkrb5-dev
  * Update build-depends minimums for libx11-dev (>= 2:1.0),
    libgtk2.0-dev (>= 2.12), zlib1g-dev (>= 1:1.2.3), libpng12-dev (>= 1.2.0),
    libjpeg62-dev (>= 6b), libcairo2-dev (>= 0.5.8), libgnome2-dev (>= 2.16),
    libgnomevfs2-dev (>= 1:2.16), libgnomeui-dev (>= 2.16),
    libnss3-dev (>= 3.12.0~1.9b3)
  * Bump standards version to 3.8.0
  * Replace ${Source-Version} by ${binary:Version} in control file
    - update debian/control
  * Bump requirement for system nspr to >= 4.8 since Mozilla bug 492464 landed
  * Bump requirement for system nss to >= 3.12.3 since Mozilla bug 485052 landed
  * Use in-source hunspell when hunspell 1.2 is not available
  * Add conditionnal support for --with-libxul-sdk controlled by
    $(USE_SYSTEM_XUL)
    - update debian/rules
  * Add p...

Read more...

Changed in thunderbird (Ubuntu):
status: Triaged → Fix Released
Changed in thunderbird:
importance: Unknown → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.