STR: 1. On a Galaxy Note, load https://support.mozilla.org/en-US/mobile 2. Type some words in the text form 3. Try to select a word using the Galaxy Note's stylus AR: Some or all of the word's characters will be deleted. ER: The word should be selected. This bug may be related to bug 750511 and/or bug 755599.
blocking-fennec1.0: --- → ?
status-firefox14: --- → affected
status-firefox15: --- → affected
This bug only happens with the stylus.
status-firefox16: --- → affected
Created attachment 635106 [details] [diff] [review] ignore-spurious-stylus-events.patch Ignore spurious stylus events on Gingerbread Galaxy Note.
Assignee: nobody → cpeterson
Status: NEW → ASSIGNED
Attachment #635106 - Flags: review?(blassey.bugs)
This problem is also mentioned on Stack Overflow: * "Samsung Galaxy Note - Why KeyEvent.KEYCODE_FORWARD_DEL events are generated when its stylus touches/releases the screen?" http://stackoverflow.com/questions/9882789/samsung-galaxy-note-why-keyevent-keycode-forward-del-events-are-generated-when
perhaps we should ignore all events (on all devices) where (event.getMetaState() & KeyEvent.FLAG_TRACKING) is true. If I read the docs right, that means the system is still tracking those key events for a potential long press which we'd get as a different event.
(In reply to Brad Lassey [:blassey] from comment #4) > perhaps we should ignore all events (on all devices) where > (event.getMetaState() & KeyEvent.FLAG_TRACKING) is true. If I read the docs > right, that means the system is still tracking those key events for a > potential long press which we'd get as a different event. Filtering all FLAG_TRACKING events make sense, but it makes me a little nervous. What if we filter all FLAG_TRACKING events in Fennec 16, but keep the Gingerbread Galaxy Note check for Fennec 14[.1]? For Fennec 15, I don't have strong feelings one way or the other.
blassey, I was mistaken. FLAG_TRACKING is a bit mask for KeyEvent.getFlag(), NOT KeyEvent.getMetaState(). The spurious stylus events on the Gingerbread Galaxy Note return getMetaState() == 512 (META_ALT_LOCKED), which is totally bogus value. KeyEvent.metaStateHasModifiers() says so here: https://github.com/android/platform_frameworks_base/blob/6651a638348c15e89e265b0a53c775cac9beafa2/core/java/android/view/KeyEvent.java#L2040 Perhaps the safest and most generic fix would be to ignore KeyEvents whose getMetaState() has unexpected bits set.
Created attachment 635829 [details] [diff] [review] part-1-ignore-spurious-stylus-events.patch Part 1: Ignore spurious key events with invalid metaState flags.
Created attachment 635830 [details] [diff] [review] part-2-but-only-on-Gingerbread-Galaxy-Note.patch Part 2: Only ignore spurious key events on Gingerbread Galaxy Note. If we want to be conservative, we can restrict Fennec 14.1 (and maybe 15) to only filter bogus KeyEvents on Gingerbread Galaxy Note.
Attachment #635830 - Flags: review?(blassey.bugs)
@blassey, can you take a look at the attached patches? I would like to get this fix into Fennec 14.1 because this bug deletes text the user has already typed, which is pretty frustrating.
Comment on attachment 635829 [details] [diff] [review] part-1-ignore-spurious-stylus-events.patch talked to chris on irc, new patch incoming
Attachment #635829 - Flags: review?(blassey.bugs) → review-
Attachment #635830 - Flags: review?(blassey.bugs) → review-
Created attachment 636855 [details] [diff] [review] ignore-unknown-keycodes.patch Ignore KeyEvents with keyCodes greater than getMaxKeyCode(). This is a more generic fix than the one we discussed in IRC, but I think it's hard to argue that we should process keyCodes > MaxKeyCode. :) Gingerbread Galaxy Note's getMaxKeyCode() returns 110. The stylus hover events have keycode 112 (KEYCODE_FORWARD_DEL) and the stylus button events have keycode 114 (KEYCODE_CTRL_RIGHT). Those keycodes were introduced in Honeycomb, so a Gingerbread device should not be generating them. ICS Galaxy Note's getMaxKeyCode() returns 225, which is strange because Android's ICS source code says getMaxKeyCode() is 210. After the Galaxy Note has been updated to ICS, the stylus no longer generates the problematic hover events. https://github.com/android/platform_frameworks_base/blob/android-4.0.4_r2.1/core/java/android/view/KeyEvent.java#L595
Attachment #636855 - Flags: review?(blassey.bugs) → review+
status-firefox16: affected → fixed
Comment on attachment 636855 [details] [diff] [review] ignore-unknown-keycodes.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): OS bug on Galaxy Note User impact if declined: Galaxy Note users can inadvertently delete typed text with stylus. This is very frustrating when typing with stylus, a feature Samsung promotes. Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): Low risk. We are ignoring key events with invalid key codes, which would only be generated by an OS bug or undocumented hardware events (like Galaxy Note's stylus). I'd like to get this fix into Fennec 14.0.1 and 15. String or UUID changes made by this patch:
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Brian, I'll be on PTO next week. Do you mind uplifting this patch to Aurora and Beta after its approved? I'd like to get this fix into 14.0.1.
Assignee: cpeterson → bnicholson
Comment on attachment 636855 [details] [diff] [review] ignore-unknown-keycodes.patch [Triage comment] Low risk fix with a high impact on usability - approving for aurora and mozilla-beta (tip of repo) so it can go out in 14.0.1
status-firefox14: affected → fixed
status-firefox15: affected → fixed
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
status-firefox16: fixed → verified
status-firefox17: --- → verified
You need to log in before you can comment on or make changes to this bug.