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)
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
People
(Reporter: jbos, Assigned: masayuki)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete)
Attachments
(3 files, 2 obsolete files)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Updated•14 years ago
|
Summary: When Google VKB is opened, Key repeat does not work! → When Google VKB is opened, Key-Repeat does not work!
Reporter | ||
Updated•14 years ago
|
tracking-fennec: --- → ?
Reporter | ||
Updated•14 years ago
|
Comment 1•14 years ago
|
||
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
Hardware: x86 → All
Comment 2•14 years ago
|
||
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/
Updated•14 years ago
|
Component: General → Event Handling
Product: Fennec → Core
QA Contact: general → events
Comment 3•14 years ago
|
||
pretty sure this is a platform bug
Assignee | ||
Comment 4•14 years ago
|
||
I don't understand this report, do you think that this bug is caused by bug 340149?
Assignee | ||
Comment 5•14 years ago
|
||
Looks like there is no problem on Linux (Ubuntu 9.10) build too. What's this about?
Comment 6•14 years ago
|
||
(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.
Assignee | ||
Comment 7•14 years ago
|
||
Oh, I can reproduce this bug on Ubuntu 10.4.
Comment 8•14 years ago
|
||
WONTFIX?
Comment 9•14 years ago
|
||
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.
Reporter | ||
Comment 10•14 years ago
|
||
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.
Assignee | ||
Comment 11•14 years ago
|
||
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
Assignee | ||
Comment 12•14 years ago
|
||
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)
Assignee | ||
Comment 13•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
Attachment #477159 -
Flags: review?
Assignee | ||
Updated•14 years ago
|
Attachment #477138 -
Attachment is obsolete: true
Attachment #477138 -
Flags: review?(karlt)
Assignee | ||
Comment 15•14 years ago
|
||
Comment on attachment 477159 [details] [diff] [review]
Patch v1.0.1
fix related comment.
Attachment #477159 -
Flags: review? → review?(karlt)
Assignee | ||
Comment 16•14 years ago
|
||
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 17•14 years ago
|
||
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+
Assignee | ||
Comment 18•14 years ago
|
||
(In reply to comment #17)
> Can you remove the now unused mKeyDownFlags and related code, please?
Okay.
Assignee | ||
Comment 19•14 years ago
|
||
Attachment #477159 -
Attachment is obsolete: true
Attachment #477397 -
Flags: review?(Olli.Pettay)
Attachment #477159 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 20•14 years ago
|
||
Assignee | ||
Comment 21•14 years ago
|
||
Reporter | ||
Comment 22•14 years ago
|
||
can you take a look on qt port too? or should someone else make the patch for qt
Assignee | ||
Comment 23•14 years ago
|
||
(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.
Assignee | ||
Comment 24•14 years ago
|
||
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)
Comment 25•14 years ago
|
||
IMO, it would be very scary to take this kind of change to branches.
Comment 26•14 years ago
|
||
...but reviewing...
Assignee | ||
Comment 27•14 years ago
|
||
(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...
Comment 28•14 years ago
|
||
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.).
Updated•14 years ago
|
Attachment #477397 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 29•14 years ago
|
||
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?
Comment 30•14 years ago
|
||
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: ? → -
Comment 31•14 years ago
|
||
(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.
Updated•14 years ago
|
blocking2.0: ? → beta8+
Comment 32•14 years ago
|
||
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...
Comment 33•14 years ago
|
||
Also see bug 517495, which reports a different behavior.
Updated•14 years ago
|
Updated•14 years ago
|
Whiteboard: [needs landing]
Assignee | ||
Comment 34•14 years ago
|
||
Comment on attachment 477397 [details] [diff] [review]
Patch v1.1
canceling a2.0? due to beta8+.
Attachment #477397 -
Flags: approval2.0?
Assignee | ||
Comment 35•14 years ago
|
||
(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+ → ?
Updated•14 years ago
|
blocking2.0: ? → beta8+
Assignee | ||
Comment 36•14 years ago
|
||
Oh, sorry for resetting the flags unexpectedly.
Would -'ing the blocking 1.9.2 and the blocking 1.9.1?
Updated•14 years ago
|
tracking-fennec: ? → 2.0b2+
Updated•14 years ago
|
blocking1.9.1: ? → -
blocking1.9.2: ? → -
Assignee | ||
Comment 37•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Assignee | ||
Updated•14 years ago
|
Keywords: dev-doc-complete
Assignee | ||
Comment 38•14 years ago
|
||
Updated•14 years ago
|
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in
before you can comment on or make changes to this bug.
Description
•