If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[fig] Scrolling on search suggestions page does not dismiss the keyboard

RESOLVED FIXED in Firefox 26

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mcomella, Assigned: mcomella)

Tracking

Trunk
Firefox 26
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-fig)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

4 years ago
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

4 years ago
Priority: -- → P1
(Assignee)

Comment 1

4 years ago
Created attachment 789695 [details] [diff] [review]
Patch

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 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

4 years ago
Created attachment 790519 [details] [diff] [review]
01a: Patch

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 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

4 years ago
Created attachment 790828 [details] [diff] [review]
01b: Patch

Fix nits, move r+.
Attachment #790519 - Attachment is obsolete: true
Attachment #790828 - Flags: review+
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

4 years ago
Created attachment 790837 [details] [diff] [review]
01c: Patch

Whoops! Sorry! x_x

Not moving my r+ this time!
Attachment #790828 - Attachment is obsolete: true
Attachment #790837 - Flags: review?(lucasr.at.mozilla)
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

4 years ago
https://hg.mozilla.org/projects/fig/rev/ed952cface7e
Status: NEW → ASSIGNED
Whiteboard: fixed-fig
https://hg.mozilla.org/mozilla-central/rev/ed952cface7e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.