274440 - ".key" should not be used to describe access keys, use ".accesskey" instead

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, defect)

Description

[Robert Kaiser]

In bug 195492, you introduced a few entities in pageInfo.dtd that end in ".key"
but are used for access keys. ".key" is normally used for hotkeys in menus
though, and loclizizers normally leave them untouched, whereas ".accesskey"
entitites normally are used for access keys and get translated so they fit to
the localized string.

currenty, it looks to me that "selectall.key", "copy.key" and eventually
"closeWindow.key" (not sure about that one) shoiuld be changed to the
".accesskey" variant.
That's very simple to fix but should help localizers a great bit.

Comment 1

[[e-mail]]

I'm not sure where selectall comes from but copy and close window are hot keys.

Comment 2

[Robert Kaiser]

Erm, I'm not sure if I used the term "hot keys" correctly, I meant to
distinguish between access keys (underlined letters in the context menu and
other areas, usually triggered by Alt+accesskey when item is visible) and
shortcut keys (key combination that always triggers this function even if item
not visible in an open menu, usually by Ctrl+key), and used "hot keys" for the
latter.

I'm sure that at least selectall.key and copy.key are access keys, not shortcut
keys here (verified by localizing them).

Comment 3

[Robert Kaiser]

Ah, now, looking at the code, I begin to figure out the real problem.

So closeWindow.key is OK, the two other entities are actually used for both key
and accesskey, so that should be split into two seperate entities for each, as
loclizers should be able to localize the accesskey without changing the shortcut
key...

Comment 4

[u60234]

Attachment: proposed patch

Comment 5

[Daniel Brooks [:db48x]]

Comment on attachment 174078 [details] [diff] [review]
proposed patch

>-    <key key="&selectall.key;"   modifiers="alt"   command="cmd_selectall"/>
…
>-    <key key="&selectall.key;"   modifiers="alt"   command="cmd_selectall"/>

Don't remove those lines, they're there for a reason. Other than that, it looks
correct.

I'm not the guy to go for to get super review, Neil is. I will give you r=
though, with that one correction.

Comment 6

[u60234]

Attachment: updated patch

Don't remove the "<key key=" lines.

Comment 7

[[e-mail]]

Comment on attachment 174078 [details] [diff] [review]
proposed patch

Cancelling request on obsolete patch.

Comment 8

[[e-mail]]

Comment on attachment 174088 [details] [diff] [review]
updated patch

Transferring r=db48x as per his comment.

Comment 9

[u60234]

Could you check this in so it does not miss the l10n freeze, Daniel? 

Comment 10

[Daniel Brooks [:db48x]]

checked in