Closed
Bug 762968
Opened 13 years ago
Closed 13 years ago
tapping on search suggestions in awesomescreen takes two taps
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox15 unaffected, firefox16 verified)
VERIFIED
FIXED
Firefox 16
Tracking | Status | |
---|---|---|
firefox15 | --- | unaffected |
firefox16 | --- | verified |
People
(Reporter: joe, Assigned: bnicholson)
References
Details
Attachments
(4 files, 1 obsolete file)
5.39 KB,
patch
|
wesj
:
review+
|
Details | Diff | Splinter Review |
1.33 KB,
patch
|
cpeterson
:
review+
|
Details | Diff | Splinter Review |
1.53 KB,
patch
|
Details | Diff | Splinter Review | |
2.98 KB,
patch
|
Details | Diff | Splinter Review |
When I search, I now get search suggestions (yay!) However, when I tap on that, the first tap dismisses the keyboard, and the *second* tap actually navigates to the search suggestion. It should work with one tap.
Reporter | ||
Updated•13 years ago
|
tracking-firefox16:
--- → ?
Reporter | ||
Updated•13 years ago
|
tracking-fennec: --- → ?
status-firefox15:
--- → unaffected
status-firefox16:
--- → affected
tracking-firefox16:
? → ---
Updated•13 years ago
|
OS: Mac OS X → Android
Hardware: x86 → ARM
Assignee | ||
Comment 1•13 years ago
|
||
Removing the hideSoftInput() call in onInterceptTouchEvent() seems to fix this (but the keyboard stays up). But I don't know why hiding the soft input would interfere with children receiving events.
wesj, cpeterson: any ideas?
OS: Android → Mac OS X
Hardware: ARM → x86
Assignee | ||
Updated•13 years ago
|
OS: Mac OS X → Android
Hardware: x86 → ARM
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → bnicholson
Assignee | ||
Comment 2•13 years ago
|
||
The onKeyDown() hack was introduced in bug 697773 to fix hardware keyboard input; we should only need to use this on those phones.
Attachment #636906 -
Flags: review?(cpeterson)
Assignee | ||
Comment 3•13 years ago
|
||
As mentioned in comment 1, the hideSoftInput() call in onInterceptTouchEvent() seemed to be the cause of this behavior.
Here is what I *think* is going on:
- onInterceptTouchEvent() is called, hiding the keyboard before the touch events are sent to the suggestion view.
- Since the hiding keyboard resizes the AwesomeScreen view, this requires that it be updated
- This, in turn, calls our getView() method that populates the suggestion view and assigns them click handlers
- Since the view/handlers are recreated during the touch event, their touch state is lost
Replacing onInterceptTouchEvent() with individual handlers (i.e., reverting bug 750511) seems to fix this problem. This apparently causes the keyboard to be hidden *after* the touch events, avoiding the view creation issues mentioned above.
Bug 750511 was landed to fix issues with Galaxy Note stylus prematurely hiding the keyboard. The stylus triggers an onKeyDown() event when near the screen; we then give focus to the EditText. This triggered the onFocusChanged() listener Wes mentioned at https://bugzilla.mozilla.org/show_bug.cgi?id=750511#c13, which hides the keyboard. By making sure the onKeyDown() code is only fired for phones with hardware keyboards, the EditText won't be focused, onFocusChanged() won't be called, and the keyboard won't be hidden.
I've tested these patches on a Galaxy Note, Droid RAZR, and the Android emulator with a hardware keyboard.
Attachment #636917 -
Flags: review?(wjohnston)
Assignee | ||
Comment 4•13 years ago
|
||
Alternate approach - rather than only applying the HKB hack to HKB phones, this simply consumes the Note's stylus event.
Attachment #636906 -
Attachment is obsolete: true
Attachment #636906 -
Flags: review?(cpeterson)
Attachment #636925 -
Flags: review?(cpeterson)
Comment 5•13 years ago
|
||
Comment on attachment 636925 [details] [diff] [review]
Part 1: Ignore bogus stylus events in onKeyDown()
Review of attachment 636925 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #636925 -
Flags: review?(cpeterson) → review+
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #3)
> Replacing onInterceptTouchEvent() with individual handlers (i.e., reverting
> bug 750511) seems to fix this problem. This apparently causes the keyboard
> to be hidden *after* the touch events, avoiding the view creation issues
> mentioned above.
Correction - without onInterceptTouchEvent(), the keyboard isn't hidden at all when touching the individual suggestion items. I guess this is because we aren't touching the allPagesList view itself.
Comment 7•13 years ago
|
||
I'm not a big fan of this patch, mostly cause I hate having this many listeners. I hate that every search term has its own click listener. But Android makes this hard(er, compared to html at least).
I have an alternative for you. Tell me if you hate it, and maybe we can come to some sort of consensus....
Attachment #637233 -
Flags: review?(bnicholson)
Assignee | ||
Comment 8•13 years ago
|
||
Also not a pretty fix, but at least this one keeps a single touch listener.
Attachment #637285 -
Flags: review?(wjohnston)
Comment 9•13 years ago
|
||
Comment on attachment 637285 [details] [diff] [review]
alt fix: Prevent hiding keyboard if suggestion view is clicked
Review of attachment 637285 [details] [diff] [review]:
-----------------------------------------------------------------
Brian, you might consider extracting this complicated code into a helper function. Maybe something like `FlowLayout getSuggestionView()` and/or `boolean suggestionViewContains(MotionEvent event)`.
Comment 10•13 years ago
|
||
Comment on attachment 636917 [details] [diff] [review]
Part 2: Replace onInterceptTouchEvent() with individual listeners
Review of attachment 636917 [details] [diff] [review]:
-----------------------------------------------------------------
I hate this least.
Attachment #636917 -
Flags: review?(wjohnston) → review+
Updated•13 years ago
|
Attachment #637233 -
Flags: review?(bnicholson)
Comment 11•13 years ago
|
||
Comment on attachment 637285 [details] [diff] [review]
alt fix: Prevent hiding keyboard if suggestion view is clicked
Review of attachment 637285 [details] [diff] [review]:
-----------------------------------------------------------------
Lets take the other guy.
Attachment #637285 -
Flags: review?(wjohnston)
Assignee | ||
Comment 12•13 years ago
|
||
Comment 13•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/14bd687b39b2
https://hg.mozilla.org/mozilla-central/rev/93aa6874678d
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Updated•13 years ago
|
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
tracking-fennec: ? → ---
Updated•5 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•