Galaxy Note's stylus deletes characters when trying to select a word in a text form

VERIFIED FIXED in Firefox 14

Status

()

VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: bnicholson)

Tracking

unspecified
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox14 fixed, firefox15 verified, firefox16 verified, firefox17 verified, blocking-fennec1.0 soft)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

7 years ago
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.
(Reporter)

Updated

7 years ago
blocking-fennec1.0: --- → ?
status-firefox14: --- → affected
status-firefox15: --- → affected
blocking-fennec1.0: ? → soft
(Reporter)

Comment 1

6 years ago
This bug only happens with the stylus.
status-firefox16: --- → affected
(Reporter)

Comment 2

6 years ago
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)
(Reporter)

Comment 3

6 years ago
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.
(Reporter)

Comment 5

6 years ago
(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.
(Reporter)

Comment 6

6 years ago
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.
(Reporter)

Comment 7

6 years ago
Created attachment 635829 [details] [diff] [review]
part-1-ignore-spurious-stylus-events.patch

Part 1: Ignore spurious key events with invalid metaState flags.
Attachment #635106 - Attachment is obsolete: true
Attachment #635106 - Flags: review?(blassey.bugs)
Attachment #635829 - Flags: review?(blassey.bugs)
(Reporter)

Comment 8

6 years ago
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)
(Reporter)

Comment 9

6 years ago
@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-
(Reporter)

Comment 11

6 years ago
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 #635829 - Attachment is obsolete: true
Attachment #635830 - Attachment is obsolete: true
Attachment #636855 - Flags: review?(blassey.bugs)
Attachment #636855 - Flags: review?(blassey.bugs) → review+
(Reporter)

Comment 13

6 years ago
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:
Attachment #636855 - Flags: approval-mozilla-beta?
Attachment #636855 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d70b14d6df33
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
(Reporter)

Comment 15

6 years ago
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
Attachment #636855 - Flags: approval-mozilla-beta?
Attachment #636855 - Flags: approval-mozilla-beta+
Attachment #636855 - Flags: approval-mozilla-aurora?
Attachment #636855 - Flags: approval-mozilla-aurora+

Updated

6 years ago
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.