Closed
Bug 1158906
Opened 9 years ago
Closed 9 years ago
Pressing <enter> on hardware keyboard does not perform search
Categories
(Firefox for Android Graveyard :: Search Activity, defect)
Tracking
(firefox39 fixed, firefox40 fixed)
RESOLVED
FIXED
Firefox 40
People
(Reporter: toonetown, Assigned: toonetown)
References
Details
Attachments
(1 file, 1 obsolete file)
1.75 KB,
patch
|
jchen
:
review+
Margaret
:
feedback+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
When searching using the search widget, the software keyboard shows a "search" key instead of an "enter" key. However, performing the same search with a hardware keyboard (for example, in the android emulator with hardware keyboard enabled), the action that gets sent is an "enter" only. I believe that this is due to the listener only looking for EditorInfo.IME_ACTION_SEARCH on line ~87 of org.mozilla.search.autocomplete.SearchBar.java. I don't know what the input system is actually sending when you press the hardware <enter> key - looking at the list of IME_ACTION_* in the android docs, I would guess it's probably IME_ACTION_UNSPECIFIED. I am not very familiar with how the android input system works, but perhaps it could be fixed by adding some kind of IME_MASK_ACTION on the enter key?
Assignee | ||
Comment 1•9 years ago
|
||
Patch which changes the logic so that it checks for the software keyboard search action (EditorInfo.IME_ACTION_SEARCH) as well as the "default" action which is sent by hardware keyboards (EditorInfo.IME_ACTION_UNSPECIFIED). This makes the emulator's hardware keyboard work to perform searches by pressing <enter>
Assignee: nobody → nathan
Attachment #8598205 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 2•9 years ago
|
||
Try server https://treeherder.mozilla.org/#/jobs?repo=try&revision=3df3b02edd09
Assignee | ||
Comment 3•9 years ago
|
||
Updated patch to remove the unneeded import statement
Attachment #8598205 -
Attachment is obsolete: true
Attachment #8598205 -
Flags: review?(margaret.leibovic)
Attachment #8598209 -
Flags: review?(margaret.leibovic)
Comment 4•9 years ago
|
||
Comment on attachment 8598209 [details] [diff] [review] bug-1158906.diff This looks reasonable to me, but jchen knows more about IME, so I think he should take a look as well.
Attachment #8598209 -
Flags: review?(nchen)
Attachment #8598209 -
Flags: review?(margaret.leibovic)
Attachment #8598209 -
Flags: feedback+
Assignee | ||
Comment 5•9 years ago
|
||
I know nothing of IME either...so I'm willing to take a different approach if needed.
Comment 6•9 years ago
|
||
Comment on attachment 8598209 [details] [diff] [review] bug-1158906.diff Review of attachment 8598209 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!
Attachment #8598209 -
Flags: review?(nchen) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3f2af35fb884
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/3f2af35fb884
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 40
Comment 10•9 years ago
|
||
Bug 1172347 found that this is broken on Fx38 (already shipped). Can we try to uplift this to Fx39?
Comment 11•9 years ago
|
||
Comment on attachment 8598209 [details] [diff] [review] bug-1158906.diff Approval Request Comment [Feature/regressing bug #]: bug 1172347 [User impact if declined]: Broken <enter> for physical keyboards [Describe test coverage new/current, TreeHerder]: Patch has been on Fx40 for a while and has no regressions [Risks and why]: Low risk change with a narrow code-change scope [String/UUID change made/needed]:none
Attachment #8598209 -
Flags: approval-mozilla-beta?
Comment 12•9 years ago
|
||
Comment on attachment 8598209 [details] [diff] [review] bug-1158906.diff Approved for uplift to beta so that search will work on phones with physical keyboards. Recent regression.
Attachment #8598209 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-release/rev/6e88bd2e3743
status-firefox38.0.5:
--- → fixed
Updated•9 years ago
|
status-firefox38.0.5:
fixed → ---
status-firefox39:
--- → fixed
Updated•6 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
•