Closed Bug 1263389 Opened 8 years ago Closed 8 years ago

Support the .com button on the virtual keyboard

Categories

(Firefox :: Address Bar, defect)

All
Windows
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox45 --- unaffected
firefox46 --- unaffected
firefox47 --- unaffected
firefox48 + verified
firefox49 --- verified

People

(Reporter: mossop, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

The windows 10 virtual keyboard has a .com key when our url bar is selected but it doesn't do anything.
Err, this works for me... can you provide more exact steps to reproduce ?
Flags: needinfo?(dtownsend)
Blocks: windows-10
Component: Tabbed Browser → Location Bar
Depends on: 1239319
(In reply to :Gijs Kruitbosch from comment #1)
> Err, this works for me... can you provide more exact steps to reproduce ?

Umm sure.

1. Tap on the address bar when in tablet mode. The virtual keyboard should pop up.
2. Press the ".com" key.
3. Nothing happens.
Flags: needinfo?(dtownsend)
Is this Nightly? Beta? Do neither work? Windows 10 general release or insider build (and if so, what version) ? 

This is generally weird because it's technically windows' job to send us the ".com" string, and so I have no idea why it would (a) not work or more confusingly (b) work for you but not me...

Do you have the same problem with Chrome?

If you use the machine while a physical keyboard is connected, and turn off ui.osk.detect_physical_keyboard in about:config, does it work when the OSK come up at that stage?
Flags: needinfo?(dtownsend)
(In reply to :Gijs Kruitbosch from comment #3)
> Is this Nightly? Beta? Do neither work? Windows 10 general release or
> insider build (and if so, what version) ? 

Nightly on general release Windows 10.

> This is generally weird because it's technically windows' job to send us the
> ".com" string, and so I have no idea why it would (a) not work or more
> confusingly (b) work for you but not me...
> 
> Do you have the same problem with Chrome?
>
> If you use the machine while a physical keyboard is connected, and turn off
> ui.osk.detect_physical_keyboard in about:config, does it work when the OSK
> come up at that stage?

I'll try to test this later.
(In reply to Dave Townsend [:mossop] from comment #4)
> > If you use the machine while a physical keyboard is connected, and turn off
> > ui.osk.detect_physical_keyboard in about:config, does it work when the OSK
> > come up at that stage?
> 
> I'll try to test this later.

I see the same issue. Another issue I see is that if I put on caps lock on the virtual keyboard no keypresses work at all
Flags: needinfo?(dtownsend)
It works fine in Chrome
I have no idea why this would happen or how to debug it. :-\

Jared or Masayuki, do you have any clues as to what might be causing this?
Flags: needinfo?(masayuki)
Flags: needinfo?(jaws)
Which path uses to input ".com"? If it's TSF path, we can get the behavior log with |NSPR_LOG_MODULES=nsTextStoreWidgets:5| and |NSPR_LOG_FILE=| with path to log file.

Otherwise, if it's generated with key/char messages, we don't have logging code...

If the latter case, I wonder, depending on active keyboard layout?
Flags: needinfo?(masayuki)
I can reproduce this as well as all of the emoji keys, caps lock, and accented characters via long press.

I tried setting those environment variables via Windows' System > Advanced System Settings > Environment Variables... as well as through the bash `export NSPR_LOG_MODULES=nsTextStoreWidgets:5` and `export NSPR_LOG_FILE=/c/nspr.log` command, but nspr.log never got created.

Are these keys sent differently than regular keyboard keys? It seems weird that I can successfully send a capital A character but not the same character when caps lock is enabled.
Flags: needinfo?(jaws)
I wanted to look at the Windows Messages and tried using Spy++ on the on-screen keyboard but Spy++ doesn't see it as a window. I'm guessing it is a Metro app. Jim, do you have any tips for debugging this?
Flags: needinfo?(jmathies)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> `export
> NSPR_LOG_FILE=/c/nspr.log` command, but nspr.log never got created.

I expect you'll need a Windows path for this...
I used a Windows path in the System environment variable setting and a bash path in the bash environment variable.
These characters are coming in as unicode using the VK_PACKET virtual key code. Looks like we don't have support for this yet. Masayuki, and sense of how much work this might be? Can gecko keyboard events handle delivering unicode characters?

http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.cpp#3097
Flags: needinfo?(jmathies) → needinfo?(masayuki)
(In reply to Jim Mathies [:jimm] from comment #13)
> These characters are coming in as unicode using the VK_PACKET virtual key
> code. Looks like we don't have support for this yet. Masayuki, and sense of
> how much work this might be? Can gecko keyboard events handle delivering
> unicode characters?
> 
> http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.
> cpp#3097

I think that it should be treated as composition. How does it works with other browsers?
https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #14)
> (In reply to Jim Mathies [:jimm] from comment #13)
> > These characters are coming in as unicode using the VK_PACKET virtual key
> > code. Looks like we don't have support for this yet. Masayuki, and sense of
> > how much work this might be? Can gecko keyboard events handle delivering
> > unicode characters?
> > 
> > http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.
> > cpp#3097
> 
> I think that it should be treated as composition. How does it works with
> other browsers?
> https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html

null data for both the emoticons and the .com key on this test page.

0 0 0 x x x x Unidentified 0 false - - ''
(In reply to Jim Mathies [:jimm] from comment #15)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #14)
> > (In reply to Jim Mathies [:jimm] from comment #13)
> > > These characters are coming in as unicode using the VK_PACKET virtual key
> > > code. Looks like we don't have support for this yet. Masayuki, and sense of
> > > how much work this might be? Can gecko keyboard events handle delivering
> > > unicode characters?
> > > 
> > > http://mxr.mozilla.org/mozilla-central/source/widget/windows/KeyboardLayout.
> > > cpp#3097
> > 
> > I think that it should be treated as composition. How does it works with
> > other browsers?
> > https://dvcs.w3.org/hg/d4e/raw-file/tip/key-event-test.html
> 
> null data for both the emoticons and the .com key on this test page.
> 
> 0 0 0 x x x x Unidentified 0 false - - ''

^^ nightly.

In IE:

1) .com translates into a series of keystrokes matching up with the .com text.
2) pizza slice translates into the pizza slice character under DOM3 :) The text input support displaying this character as well. I'll post a screen shot.
Flags: needinfo?(masayuki)
Thank you for your test. Could you check with Chrome too?

Anyway, emoji should be separated to another bug since IIRC, our editor doesn't support non-BMP character input via keypress events.
Flags: needinfo?(masayuki) → needinfo?(jmathies)
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #18)
> Thank you for your test. Could you check with Chrome too?
> 
> Anyway, emoji should be separated to another bug since IIRC, our editor
> doesn't support non-BMP character input via keypress events.

Chrome has the same behavior as IE.
Flags: needinfo?(jmathies)
So, I continued to be surprised by this because I couldn't reproduce. I tried again, and I still can't reproduce on 46 (beta) - but I can reproduce on 48. So it seems like a regression?
It seems like 5 different bugs landed in one push to inbound, so I don't know which one it was - but we clearly broke this when it used to work. :-\
yep, I can confirm, this is working on aurora.
I mean, I got a regression window, so I don't think we need these keywords... This push: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=eb7c36e2ef5d simply has a bunch of bugs in it. Considering they were pushed together I doubt that they build/function when we pull them apart. I would also expect that maybe Masayuki can tell us why they broke this?
Flags: needinfo?(masayuki)
Assignee: nobody → masayuki
No longer blocks: 1137563, 1137565, 1137572
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
OS: Unspecified → Windows
Hardware: Unspecified → All
Version: unspecified → Trunk
TextEventDispatcher initializes charCode value with mKeyValue (and mKeyNameIndex) of WidgetKeyboardEvent.  Therefore, NativeKey needs to initialize mKeyValue properly at handling WM_KEYDOWN message of VK_PACKET.  However, nobody initializes mCommittedCharsAndModifiers value of VK_PACKET.  Additionally, KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex() returns KEY_NAME_INDEX_Unidentified for it too.

Therefore, this patch creates a path for handling VK_PACKET.  First, makes KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex() returns KEY_NAME_INDEX_USE_STRING.  Next, the constructor of NativeKey initializes mCommittedCharsAndModifiers with following char message.  Additionally, makes sure that VK_PACKET is always handled with following char message even if there is no char message.

Review commit: https://reviewboard.mozilla.org/r/48085/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48085/
Attachment #8743873 - Flags: review?(m_kato)
Comment on attachment 8743873 [details]
MozReview Request: Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of  WM_KEYDOWN of VK_PACKET with following char message r?m_kato

https://reviewboard.mozilla.org/r/48085/#review45431
Attachment #8743873 - Flags: review?(m_kato) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/14a02164b038bddd89b96c33b6932f069b8371ff
Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of  WM_KEYDOWN of VK_PACKET with following char message r=m_kato
https://hg.mozilla.org/mozilla-central/rev/14a02164b038
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8743873 [details]
MozReview Request: Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of  WM_KEYDOWN of VK_PACKET with following char message r?m_kato

Approval Request Comment
[Feature/regressing bug #]: regression of bug 1137561.
[User impact if declined]: Users cannot use some keys on virtual keyboard and some a11y tools which use SendInput() API to input unicode characters.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Low. Simply, creating a path for VK_PACKET pseudo key event which is sent when a unicode character is inputted without active keyboard layout.
[String/UUID change made/needed]: Nothing.
Attachment #8743873 - Flags: approval-mozilla-aurora?
Comment on attachment 8743873 [details]
MozReview Request: Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of  WM_KEYDOWN of VK_PACKET with following char message r?m_kato

Improve virtual keyboard support, has baked in Nightly for over a week, Aurora48+
Attachment #8743873 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This issue is also reproducing on Windows 8.1 x64 using 48.0a1 (2016-04-09).

I verified it on:
- Windows 10 Home Insider Preview x64 
- Windows 8.1 x64   
using:
- Latest Nightly 49.0a1 (2016-05-05)
- Latest Aurora 48.0a2 (2016-05-05)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.