Closed Bug 597981 Opened 14 years ago Closed 14 years ago

When Google VKB is opened, Key-Repeat does not work! (On latest gtk2 platforms, keydown events are not dispatched by auto repeat)

Categories

(Core :: Widget: Gtk, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta8+
blocking1.9.2 --- -
blocking1.9.1 --- -
fennec 2.0b2+ ---

People

(Reporter: jbos, Assigned: masayuki)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 2 obsolete files)

STEPS LEADING TO PROBLEM: 

1. Open google.com page, HWK is active.
2. Open Google VKB 
3. Press and keep any normal button on HKW
4. Look at the google input filed

EXPECTED OUTCOME:
When button is pressed and kept character should be repeated as long as is
pressed.

ACTUAL OUTCOME:
It requires pressing and releasing every time new character should be entered,
so automatic repetition when key is pressed for longer time doesn't work in
Browser when google VKB is opened. 

FREQUENCY OF OCCURRENCE: 
always
Blocks: 583135
Depends on: 577630, 591891
Summary: When Google VKB is opened, Key repeat does not work! → When Google VKB is opened, Key-Repeat does not work!
tracking-fennec: --- → ?
Blocks: 583150
No longer blocks: 583135
This might not be a Fennec bug; I cannot get key repeat to work while Google Virtual Keyboard is displayed in Minefield either:

Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre
Key repeat using the hardware keyboard on the Google Virtual Keyboard demo works in both Chromium and Opera, but not Firefox 3.6 or Firefox 4 nightlies:
http://www.google.com/webelements/virtualkeyboard/
Component: General → Event Handling
Product: Fennec → Core
QA Contact: general → events
pretty sure this is a platform bug
I don't understand this report, do you think that this bug is caused by bug 340149?
Looks like there is no problem on Linux (Ubuntu 9.10) build too. What's this about?
(In reply to comment #2)
> Key repeat using the hardware keyboard on the Google Virtual Keyboard demo
> works in both Chromium and Opera, but not Firefox 3.6 or Firefox 4 nightlies:
> http://www.google.com/webelements/virtualkeyboard/

Sounds like a bug in the web app.
Oh, I can reproduce this bug on Ubuntu 10.4.
WONTFIX?
well, someone should look at what the webapp is doing and then
we need to figure out if the bug is in gecko or in the web app.
I have the impression that its releated to this bug 577630. So maybe we need to figure out whats going on with this and see if it fixes this issue. For my opinion - with that bug we just see a inconsistency in handling the keyevents in the system.
Okay, probably, this is a behavior change of GTK2 or IM. I'll post a patch.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Component: Event Handling → Widget: Gtk
QA Contact: events → gtk
Attached patch Patch v1.0 (obsolete) — Splinter Review
The cause of this bug is, Ubuntu 10.4 doesn't send native keyrelease event during pressing a key but Ubuntu 9.10 sent it. Therefore, the dispatched DOM key events are changed at auto repeat.
9.10:  KEYDOWN -> KEYPRESS -> KEYUP -> KEYDOWN...
10.4:  KEYDOWN -> KEYPRESS -> KEYPRESS -> KEYPRESS...

However, our KeyDown event must be dispatched when native keypress event is received. (DOM3 event spec said so, and our editor listens keydown event)

Note that the keydown flags can be removed on current build, but I think that we shouldn't do it because DOM3 key events have repeat property. When we'll implement it, the flags is needed.
Attachment #477138 - Flags: review?(karlt)
Well, but I'm not sure why the software keyboard kills the auto repeat.

And following test case doesn't work fine on current trunk build on Ubuntu10.4.
> data:text/html,<script type="text/javascript">function putlog(event) { var log = document.getElementById("log"); log.innerHTML = event.type + "\n" + log.innerHTML; }</script><textarea onkeydown="putlog(event); event.preventDefault();" onkeypress="putlog(event);"></textarea><pre id="log"></pre>

At keydown event, this calls preventDefault(), then, the next keypress event doesn't cause text input. The patch also fixes this bug.
Attached patch Patch v1.0.1 (obsolete) — Splinter Review
Attachment #477159 - Flags: review?
Attachment #477138 - Attachment is obsolete: true
Attachment #477138 - Flags: review?(karlt)
Comment on attachment 477159 [details] [diff] [review]
Patch v1.0.1

fix related comment.
Attachment #477159 - Flags: review? → review?(karlt)
Comment on attachment 477159 [details] [diff] [review]
Patch v1.0.1

The new behavior should be reviewed by Smaug too.
Attachment #477159 - Flags: review?(Olli.Pettay)
Comment on attachment 477159 [details] [diff] [review]
Patch v1.0.1

Someone went to some trouble to preserve the single keydown behavior, but
http://www.w3.org/TR/DOM-Level-3-Events/#events-keyboard-event-order is clear that it wants multiple keydowns and it sounds like this is what's provided for win32, so this looks fine to me.

Can you remove the now unused mKeyDownFlags and related code, please?
Attachment #477159 - Flags: review?(karlt) → review+
(In reply to comment #17)
> Can you remove the now unused mKeyDownFlags and related code, please?

Okay.
Attached patch Patch v1.1Splinter Review
Attachment #477159 - Attachment is obsolete: true
Attachment #477397 - Flags: review?(Olli.Pettay)
Attachment #477159 - Flags: review?(Olli.Pettay)
can you take a look on qt port too? or should someone else make the patch for qt
(In reply to comment #22)
> can you take a look on qt port too? or should someone else make the patch for
> qt

Can I build firefox (not fennec) with qt? If so, I'll look the qt code. Otherwise, I cannot do it.
Smaug, would you review the patch?
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Summary: When Google VKB is opened, Key-Repeat does not work! → When Google VKB is opened, Key-Repeat does not work! (On latest gtk2 platforms, keydown events are not dispatched by auto repeat)
IMO, it would be very scary to take this kind of change to branches.
...but reviewing...
(In reply to comment #25)
> IMO, it would be very scary to take this kind of change to branches.

But the branch's behavior has been changed by the platform's behavior change...
The patch will change our gtk widget code to dispatch keydown for the repeated
key events. That is rather major change. The repeated key event handling has been
there since 2001.
The change in behavior is good (and it makes gtk to work the same way as
windows and OSX, right? Please test.).
Attachment #477397 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 477397 [details] [diff] [review]
Patch v1.1

GTK2 has changed its key event spec. The old GTK2 sent a set of keypress event -> keyrelease event for every repeated key input. Therefore, we dispatched a set of DOMKeydown -> DOMKeypress -> DOMKeyup for every input.

However, current GTK2 platform sends only keypress event for every key input. Therefore, we dispatched only DOMKeypress event for input. Therefore, web apps cannot cancel the input by DOMKeydown.preventDefault() and some web application may be broken by this behavior change.

This patch dispatches DOMKeydown event before DOMKeypress event. Therefore, builds with this patch dispatch a set of DOMKeydown -> DOMKeypress for every input. This behavior conforms with DOM3 key event spec.
Attachment #477397 - Flags: approval2.0?
Scary change to compensate for a regression that's not our fault -- definitely not blocking branch releases. Once this has landed on trunk and gotten reasonable exposure and testing (how many gtk2 beta users do we have?) we might consider this for the older branches. Meanwhile Ubuntu users can stick with 9.10 or upgrade to Firefox 4 betas (after the fix lands) if the behavior really bothers them.
blocking1.9.1: ? → -
blocking1.9.2: ? → -
(In reply to comment #29)
> GTK2 has changed its key event spec. The old GTK2 sent a set of keypress event
> -> keyrelease event for every repeated key input. Therefore, we dispatched a
> set of DOMKeydown -> DOMKeypress -> DOMKeyup for every input.

I'm surprised to hear that was happening on Ubuntu 9.10.
GTK has been suppressing KeyRelease events during autorepeat since at least 2.12 (2007), and the code seems to have been written expecting that.
I don't know why the Ubuntu behavior is different.
Given that bug 599887 says our behavior isn't consistent between platforms, I don't think this should block branches, or even go on them.

But now that DOM 3 events has specified this (although I don't like what it has specified), we should support it.

We should really support the .repeat attribute on key events, though...
Also see bug 517495, which reports a different behavior.
Blocks: 577630
No longer depends on: 577630
Comment on attachment 477397 [details] [diff] [review]
Patch v1.1

canceling a2.0? due to beta8+.
Attachment #477397 - Flags: approval2.0?
(In reply to comment #32)
> We should really support the .repeat attribute on key events, though...

I filed bug 600117.
blocking1.9.1: - → ?
blocking1.9.2: - → ?
blocking2.0: beta8+ → ?
Oh, sorry for resetting the flags unexpectedly.

Would -'ing the blocking 1.9.2 and the blocking 1.9.1?
tracking-fennec: ? → 2.0b2+
blocking1.9.1: ? → -
blocking1.9.2: ? → -
http://hg.mozilla.org/mozilla-central/rev/6cb6bb98e269
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Blocks: 91592
You need to log in before you can comment on or make changes to this bug.