Closed
Bug 904122
Opened 11 years ago
Closed 11 years ago
[fig] Scrolling on search suggestions page does not dismiss the keyboard
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 26
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
(Whiteboard: fixed-fig)
Attachments
(1 file, 3 obsolete files)
3.25 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
1) Launch Figgy Fennec 2) Click the url bar 3) Type some content to bring up the search suggestions page 4) Scroll through the list Expected: Keyboard is dismissed (like any other time on the awesomescreen when the url bar is selected) Actual: Keyboard is not dismissed Note that the back button will still dismiss the keyboard
Updated•11 years ago
|
Priority: -- → P1
Assignee | ||
Comment 1•11 years ago
|
||
I wanted to place requestFocus() in an if for the down event... if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { requestFocus(); } return false; But for some reason, the search engine rows do not receive a DOWN event (though history rows and the empty background space does). Would you happen to know why?
Attachment #789695 -
Flags: feedback?(lucasr.at.mozilla)
Comment 2•11 years ago
|
||
Comment on attachment 789695 [details] [diff] [review] Patch Review of attachment 789695 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/home/BrowserSearch.java @@ +248,5 @@ > return selected.onKeyDown(keyCode, event); > } > return false; > } > }); nit: add empty line here. @@ +252,5 @@ > }); > + mList.setOnTouchListener(new View.OnTouchListener() { > + @Override > + public boolean onTouch(View v, MotionEvent event) { > + v.requestFocus(); // to drop soft keyboard. to drop -> Dismiss nit: move this comment before this line. We don't usually add comments on the same line.
Attachment #789695 -
Flags: feedback?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 3•11 years ago
|
||
As per IRC, I moved the call to onInterceptTouchEvent. I created a new class extending HomeListView for this so I wasn't reduplicating any of the work for dismissing the keyboard that the HomePager does (since it also uses HomeListView). Is it worth making a comment in the code about this or is the patch sufficient?
Attachment #789695 -
Attachment is obsolete: true
Attachment #790519 -
Flags: review?(lucasr.at.mozilla)
Comment 4•11 years ago
|
||
Comment on attachment 790519 [details] [diff] [review] 01a: Patch Review of attachment 790519 [details] [diff] [review]: ----------------------------------------------------------------- Looks good (with suggested change). ::: mobile/android/base/home/HomeSearchListView.java @@ +13,5 @@ > + > +/** > + * HomeSearchListView is a list view for displaying search engine results on the awesome screen. > + */ > +public class HomeSearchListView extends HomeListView { This is not big enough to deserve its own file, imo. This is very specific to BrowserSearch anyway. I'd turn this into a public inner static class in BrowserSearch. @@ +27,5 @@ > + public boolean onInterceptTouchEvent(MotionEvent event) { > + if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { > + // Dismiss the soft keyboard. > + requestFocus(); > + } nit: add empty line.
Attachment #790519 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Fix nits, move r+.
Attachment #790519 -
Attachment is obsolete: true
Attachment #790828 -
Flags: review+
Comment 6•11 years ago
|
||
Comment on attachment 790828 [details] [diff] [review] 01b: Patch Review of attachment 790828 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/resources/layout/browser_search.xml @@ +12,5 @@ > android:layout_width="match_parent" > android:layout_height="wrap_content" > android:layout="@layout/home_suggestion_prompt" /> > > + <org.mozilla.gecko.home.HomeSearchListView You have to update this to something like: <view class="org.mozilla.gecko.home.BrowserSearch$HomeSearchListView" ...
Assignee | ||
Comment 7•11 years ago
|
||
Whoops! Sorry! x_x Not moving my r+ this time!
Attachment #790828 -
Attachment is obsolete: true
Attachment #790837 -
Flags: review?(lucasr.at.mozilla)
Comment 8•11 years ago
|
||
Comment on attachment 790837 [details] [diff] [review] 01c: Patch Review of attachment 790837 [details] [diff] [review]: ----------------------------------------------------------------- Cool.
Attachment #790837 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/projects/fig/rev/ed952cface7e
Status: NEW → ASSIGNED
Whiteboard: fixed-fig
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ed952cface7e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Updated•3 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
•