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?
Created attachment 8598205 [details] [diff] [review] bug-1158906.diff 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)
Created attachment 8598209 [details] [diff] [review] bug-1158906.diff Updated patch to remove the unneeded import statement
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.
I know nothing of IME either...so I'm willing to take a different approach if needed.
Comment on attachment 8598209 [details] [diff] [review] bug-1158906.diff Review of attachment 8598209 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!
Attachment #8598209 - Flags: review?(nchen) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Bug 1172347 found that this is broken on Fx38 (already shipped). Can we try to uplift this to Fx39?
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 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+
status-firefox38.0.5: --- → fixed
status-firefox38.0.5: fixed → ---
status-firefox39: --- → fixed
You need to log in before you can comment on or make changes to this bug.