607574 - Taps in the empty area of the awesome list are being registered as "Go"

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anna (Waverley)]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101025 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Android; Linux armv7l; rv:2.0b8pre) Gecko/201026 Firefox/4.0b8pre Fennec/4.0b2pre

Vertical Panning when number of entries are less than viewable area always loads the first webpage.

   

Reproducible: Always

Steps to Reproduce:
1. Make sure you have some history.
2. Click on the URL bar to start the awesome bar.
3. Start a search that will populate a set of results that is less than the available vertical list size
4. Begin panning down 

Actual Results:  
Panning down loads the first website from the list of results.

Expected Results:  
The list should not pan down. Nothing should be changed.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Sounds like the tap (to start panning) is being handled like a simple tap because the list is not pannable (not enough items)

Comment 2

[Aakash Desai [:aakashd]]

Yeah, a tap is being registered as a "Go" essentially. If you do a tap of the list when there are no results, a google search will be performed.

Good negative test!

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

The patch correct two bug:
 * the first one is the fact that a click was fired but never stopped if the area was not draggable
 * The second is the "Go" action fired when a click was happening anywhere on the awesome results

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 486368 [details] [diff] [review]
Patch

Looks ok, but I want more eyes on the review.

We want this for post-b2

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 486368 [details] [diff] [review]
Patch

>+++ b/chrome/content/bindings.xml

>+          if (target._empty == false) {

Nit: I'd prefer "if (!target._empty)".


>+++ b/chrome/content/input.js

>+    return this.tapRadius = Services.prefs.getIntPref("ui.dragThresholdX");

While we're in here, can we change this to "ui.dragThreshold" and remove the unused dragThresholdY pref?

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5) 
> >+++ b/chrome/content/input.js
> 
> >+    return this.tapRadius = Services.prefs.getIntPref("ui.dragThresholdX");
> 
> While we're in here, can we change this to "ui.dragThreshold" and remove the
> unused dragThresholdY pref?

We can't, this is an old pref used for Firefox desktop and we need to keep in sync with it. 
http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#1986

Comment 7

[Anna (Waverley)]

The bug is also reproducible when following the next steps:

    1. Click on the URL bar.
    2. Type in any phrase.
    3. Click on the search button.
    4. Click outside the list.

After step 3 the list of available search engines appears.
When tapping outside the list, the phrase typed in step 2 is searched via Google Search Engine (the first search engine from the list).
 I have not click on Google Search at all.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/7c6e117c1f6c

Comment 9

[Ayan Shah[:ashah]]

verified fixed on:

Mozilla/5.0(Android; Linux armv7l;rv2.0b8pre) Gecko/20101102 Firefox/4.0b8pre
Fennec/4.0b3pre

Comment 10

[Ayan Shah[:ashah]]

https://litmus.mozilla.org/show_test.cgi?id=13774