Command keys using upper-case non-ASCII characters are broken

VERIFIED FIXED

Status

()

Core
Internationalization
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: u60234, Assigned: ehoogeveen)

Tracking

({regression})

13 Branch
regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox15+ wontfix, firefox16+ verified, firefox17+ verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
+++ This bug was initially created as a clone of Bug #770447 +++

Starting with the trunk nightly from 2012-03-12, command keys with non-ASCII chars, like Ctrl+Ö and Ctrl+Ä, no longer works if they are defined with capital letters in the locale dtd file. Lowercase letters are fine.

Regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6737b6762eb8&tochange=5ec9524de1af
Looks like bug 492931
Blocks: 492931
(In reply to Simon Montagu from comment #1)
> Looks like bug 492931

Indeed. That was established in bug 770447 comment 24.

I imagine this is by design, so would the best course of action here be to check the existing command keys for non-ASCII characters?

Comment 3

5 years ago
Well, non-ASCII characters are totally fine command keys, IMHO.

Also, reading bug 492931, in particular bug 492931 comment 6, I'm not sure that changing the behaviour of control keys was intended. Henri?
Hmm, I think that nsXBLPrototypeHandler.cpp change should be backed out because it doesn't make sense ignoring letter case only for ASCII alphabets. And it might break some addon compatibility too.
https://hg.mozilla.org/integration/mozilla-inbound/diff/d09b4e60bb09/content/xbl/src/nsXBLPrototypeHandler.cpp
Yeah, that change should probably be undone.
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
(In reply to Axel Hecht [:Pike] from comment #3)
> Also, reading bug 492931, in particular bug 492931 comment 6, I'm not sure
> that changing the behaviour of control keys was intended. Henri?

Not intended.
Assigning this to Emanuel to get a fix on this, tracking for release so we don't ship this regression. Emanuel, thank you for contributing (and congrats on your first patch in bug 492931, as well), if you can't take on this fix please let us know so we can re-assign in time.
Assignee: nobody → emanuel.hoogeveen
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
tracking-firefox15: ? → +
tracking-firefox16: ? → +
tracking-firefox17: ? → +
Wontfixing for 15 since we're a day away from our final beta of the cycle.  If Emanuel isn't the right assignee for this can someone select another?
status-firefox15: affected → wontfix
Created attachment 653577 [details] [diff] [review]
backout as described

This is the backout described in comment 4 and comment 5.  I haven't tested it.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 492931
User impact if declined: broken shortcut keys when non-ASCII and specified in uppercase in the code
Testing completed (on m-c, etc.): none
Risk to taking this patch (and alternatives if risky): Low, I think, but I'm not the expert.
String or UUID changes made by this patch: None.
Attachment #653577 - Flags: approval-mozilla-beta?
Attachment #653577 - Flags: approval-mozilla-aurora?
Can we get a try build to test?  I'm not comfortable taking a completely untested backout on our final beta.
Landed backout on:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb1c9884c07e

Once that cycles, mozilla-inbound builds should be testable, though I'm not sure if anyone has localized builds for anything other than beta.  (And I suspect my spinning a build off beta won't help, since it won't be localized.)
Maybe, we can test it in widget/tests/test_keycodes.xul.

Updated

5 years ago
Version: unspecified → 13 Branch
(In reply to David Baron [:dbaron] from comment #11)
> Landed backout on:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cb1c9884c07e
> 
> Once that cycles, mozilla-inbound builds should be testable, though I'm not
> sure if anyone has localized builds for anything other than beta.  (And I
> suspect my spinning a build off beta won't help, since it won't be
> localized.)

Merged to m-c:
https://hg.mozilla.org/mozilla-central/rev/cb1c9884c07e
Comment on attachment 653577 [details] [diff] [review]
backout as described

So it's too late to get the info we need on beta for this, if we can get a verification on the Nightly builds I'll approve for Aurora.
Attachment #653577 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Keywords: verifyme
RE: verifyme
I don't have access to a keyboard with non-ASCII characters.

Hosse, can you please verify if this is fixed for you with Firefox 17.0a1 Nightly 2012-08-22 or newer? Thanks.
(Reporter)

Comment 16

5 years ago
Yes, uppercase letters such as Ä and Ö works fine in today's nightly. Tested on Windows and Linux.
Thanks Hasse, removing verifyme keyword. Lukas, back to you.
Keywords: verifyme
Attachment #653577 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/2fa008533f03
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox16: affected → fixed
status-firefox17: affected → fixed
Resolution: --- → FIXED
I'm actually going to mark this verified for Firefox 17 based on comment 16.

Hasse, would you mind testing tomorrow's Aurora build to confirm it's fixed for Firefox 16?
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (offline: 8/22 - 8/27) from comment #12)
> Maybe, we can test it in widget/tests/test_keycodes.xul.

Any chance I could interest you in doing that?
Flags: in-testsuite?
(Reporter)

Comment 21

5 years ago
Tested today's Aurora build on Windows and Linux. Uppercase Ä and Ö worked fine in both.
Thanks Hasse!
status-firefox16: fixed → verified
You need to log in before you can comment on or make changes to this bug.