I and Asai-san discussed for the accel keys of zoom. They are now using '+', '-' and '0'. However, we cannot use them without shift key on many keyboard layouts. It's too unusable. Therefore, for en-US keyboards, '=' key is also specified for zoom in alternative accel key. I think (and hope) that we should add other alternative keys like that for other keyboard layouts. And they can be used by localizers. Because we should not replace the original keys (+/-/0), they are in numpad too. And also in Japan, the en-US keyboard layout is used many environments, so, we cannot replace '=' too.

Oops, sorry for the spam.

Comment on attachment 313065 [details] [diff] [review] Patch v1.0.1 I'm going to land the strings tonight.

Checking in browser/locales/en-US/chrome/browser/browser.dtd; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v <-- browser.dtd new revision: 1.106; previous revision: 1.105 done Checking in toolkit/locales/en-US/chrome/mozapps/help/help.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/help/help.dtd,v <-- help.dtd new revision: 1.10; previous revision: 1.9 done

Nakano-san's patch missed source viewer part. Now, this patch has only string changes because string-freeze is close.

Comment on attachment 313554 [details] [diff] [review] Patch for source viewer part, strings only I'll land this now.

Checking in toolkit/locales/en-US/chrome/global/viewSource.dtd; /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/viewSource.dtd,v <-- viewSource.dtd new revision: 1.7; previous revision: 1.6 done

Comment on attachment 313065 [details] [diff] [review] Patch v1.0.1 r+a=me, sorry for the delay

Thank you everyone! I'll post the patch for xul part of viewsrc.

this is needed by attachment 313554 [details] [diff] [review] (that was already checked-in)

Comment on attachment 313650 [details] [diff] [review] Patch for viewsrc (xul part) Sorry I missed this.

mconnor, can you please do the approval on this one? I feel like a broken record asking every approval for tests. :)

Comment on attachment 313554 [details] [diff] [review] Patch for source viewer part, strings only (post hoc reviews and approvals)

checked-in the xul part.

regression!! 20080407_1715_firefox-3.0pre.en-US.win32.zip "Tab","hankaku/zenkaku","ctrl++/-/0","katakana/hiragana" etc. does not work, wrong behavior. try with IME mode.

(In reply to comment #14) > try with IME mode. ignore this, sorry.

backed-out

We should ignore the *dummy* key element which is added by attachment 314261 [details] [diff] [review]. The key elements are only needed by localized build, but English build also has them (the key attribute is empty).

Comment on attachment 314269 [details] [diff] [review] Patch for XBL key handler Not 100% convinced on this so punting on one of the reviews for now.

Comment on attachment 314269 [details] [diff] [review] Patch for XBL key handler The keycode and/or charcode attributes can also be used to define a key so checking for the key attribute isn't sufficient. Using <key/> without specifying a key means listen for all keys. The first patch doesn't look correct either as it appears to create listeners which zoom the page whenever any key is pressed.

(In reply to comment #20) > (From update of attachment 314269 [details] [diff] [review]) > The keycode and/or charcode attributes can also be used to define a key so > checking for the key attribute isn't sufficient. When key attr or charcode attr is existing, but that has empty value, then, we ignore the key element, is it ok? > Using <key/> without specifying a key means listen for all keys. The first > patch doesn't look correct either as it appears to create listeners which zoom > the page whenever any key is pressed. Yeah, but I don't have other idea for this issue. The additional key elements are not needed by some locale.

This checkes both key attr and charcode attr. If each attr is existing but both values are empty, the key element is ignored. So, the key element doesn't have both attrs, that can handle the all keys.

enndeakin: Would you review this?

Comment on attachment 314907 [details] [diff] [review] Patch for XBL key handler v2.0 So is the idea that <key/> behaves the same as currently where all keys are listened to, and <key key=""/> listens to no key? If so, then that seems like it should be ok, and we should document that behaviour for the <key> element. Also, please check the 'keycode' attribute as well.

(In reply to comment #24) > (From update of attachment 314907 [details] [diff] [review]) > So is the idea that <key/> behaves the same as currently where all keys are > listened to, and <key key=""/> listens to no key? Yes. > If so, then that seems like it should be ok, and we should document that > behaviour for the <key> element. O.K. Do you mean that the document is MDC? http://developer.mozilla.org/en/docs/XUL:key

Comment on attachment 316021 [details] [diff] [review] Patch for XBL key handler v2.1 Sorry for the delay; I got confused by the logic, but I see what it's doing now.

Comment on attachment 316021 [details] [diff] [review] Patch for XBL key handler v2.1 a1.9+=damons

checked-in.

Wow, attachment 314269 [details] [diff] [review] was landed at bug 359638. Because the same file was also changed by me. Sorry for such dangerous mistake. And now, the file is updated by attachment 316021 [details] [diff] [review].

updated the dev-doc: http://developer.mozilla.org/en/docs/index.php?title=XUL%3Akey&diff=71117&oldid=63358 Please modify the document by native speakers.