1180663 - Doesn't work "Zoom In" short cut key (Ctrl++) in Developer Tools window with Japanese keyboard

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect)

Description

[magicp]

User Agent: Mozilla/5.0 (Windows NT 6.3; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

1) Open Scratchpad on Firefox (ja-JP) with Japanese keyboard.
2) Zoom In short cut (Ctrl++) doesn't work. key is (;/+)


Actual results:

Doesn't work "Zoom In" short cut key (Ctrl++) in Developer Tools window (Scratchpad, WebIDE, Browser Console) with Firefox (ja-JP) and Japanese keyboard.

However, it works in Document body and Page source window.


Expected results:

Zoom In short cut key (Ctrl++)  key(;/+) works in Developer Tools window (Scratchpad, WebIDE, Browser Console) with Firefox (ja-JP) and Japanese keyboard.

Comment 1

[:Gijs (he/him)]

Masayuki-san, I know you've worked on several of these types of bugs before; any idea why there are issues on the devtools windows but not the main window?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

WidgetKeyboardEvent has "alternative char code values" if its message is NS_KEY_PRESS. XUL shortcut key handles use this if .charCode mishit any shortcut keys. Unfortunately, these information isn't accessible from JS. If shortcut keys of devtools are implemented with JS, we need to expose the information to chrome script...

https://developer.mozilla.org/en-US/docs/Gecko_Keypress_Event#Alternative_charCodes_for_internal_key_handling

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

And currently, we cannot set the information to keydown event. In my queue, I'll do that, but perhaps, late this year or next year :-(

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, if Ctrl + Shift + '+' works, devtools need alternative shortcut key entry which can be localized:

http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd?rev=9a10f7a19719&mark=593-613#593

Comment 5

[Brian Grinstead [:bgrins]]

So if I understand correctly, the main zoom in / out shortcut for web content is localized to a different key (not + / -) in Japanese builds?

The keys for devtools are specified here in XUL: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/framework/toolbox.xul?offset=0#41-56 and here in a dtd file: https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/devtools/toolbox.dtd#18-21.

We could either add a similar comment as http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd?rev=9a10f7a19719&mark=593-613#593 or maybe we should just reuse the entities there instead to make sure we have the same shortcut as for web content.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Brian Grinstead [:bgrins] from comment #5)
> So if I understand correctly, the main zoom in / out shortcut for web
> content is localized to a different key (not + / -) in Japanese builds?

Yes. It's not convenient if it needs Shift key to do zoom-in. Therefore, unshifted key's character is also registered. This trick is used for ANSI keyboard layout (i.e., Ctrl + '='). So, Japanese localized build registers '+', '=' (for ANSI keyboard layout users) and ';' (for JIS keyboard layout users).

> The keys for devtools are specified here in XUL:
> https://dxr.mozilla.org/mozilla-central/source/browser/devtools/framework/
> toolbox.xul?offset=0#41-56 and here in a dtd file:
> https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/
> browser/devtools/toolbox.dtd#18-21.

Thanks. So, Ctrl + ';' isn't supported. That is the cause of this bug.

> We could either add a similar comment as
> http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/
> browser/browser.dtd?rev=9a10f7a19719&mark=593-613#593 or maybe we should
> just reuse the entities there instead to make sure we have the same shortcut
> as for web content.

If it's not problem to depend on the firefox's DTD file, the latter idea is reasonable since no localizers need additional work for this bug.

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1180663 - Reuse entity names and values from browser.dtd for devtools zoom functionality;r=jryans

Bug 1180663 - Reuse existing fullZoom keys from browser.dtd for devtools toolbox;r=jryans

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8655533 [details]
MozReview Request: Bug 1180663 - Reuse entity names and values from browser.dtd for devtools zoom functionality;r=jryans

https://reviewboard.mozilla.org/r/17919/#review16043

I'd actually prefer to reduce our dependencies on /browser files, not add more.

By moving in that direction, we pave the way for non-Firefox things like browser.html to use our frontend more easily.

Can we allow translators to do this mapping in our .dtd too?

It looks like it's actually [`fullZoomEnlargeCmd.commandkey3`] that defines the extra key in ja locales, so we'll need another <key> no matter which translation is used.

[1]: https://transvision.mozfr.org/string/?entity=browser/chrome/browser/browser.dtd:fullZoomEnlargeCmd.commandkey3&repo=aurora

Comment 9

[Brian Grinstead [:bgrins]]

Yeah I don't see why we couldn't copy the localization comments from the other dtd file into ours.  Maybe we could even switch to matching entity names and add a comment that they should match the translation for the same entities in browser.dtd.

Comment 10

[Brian Grinstead [:bgrins]]

Comment on attachment 8655533 [details]
MozReview Request: Bug 1180663 - Reuse entity names and values from browser.dtd for devtools zoom functionality;r=jryans

Bug 1180663 - Reuse entity names and values from browser.dtd for devtools zoom functionality;r=jryans

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8655533 [details]
MozReview Request: Bug 1180663 - Reuse entity names and values from browser.dtd for devtools zoom functionality;r=jryans

https://reviewboard.mozilla.org/r/17919/#review16089

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b0883c30a406

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b0883c30a406