909979 - Off-by-one (?) error when long-clicking list items on the search suggestions screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

1) Click the awesomebar
2) Type something with matched results (e.g. go to http://apple.com, click a few links, and search "app") 
3) Long-click on the topmost non-search provider item

Expected: A context menu for the selected link
Actual: A context menu for the link one down

I noticed on my phone this issue does not occur when the search providers have not loaded yet and thus are not present (meaning the Google provider may be offsetting this count by one).

Note that clicking the bottom-most link causes a crash.

Similar bugs: bug 909539 and bug 908423. I wonder if there isn't a root cause to these problems since I don't remember this happening before - I will try to bisect.

Comment 1

[:Margaret Leibovic]

It looks like this may be fixed by Sriram's patch in bug 909539, since that affects all HomeListViews.

Comment 2

[Sriram Ramasubramanian [:sriram]]

I tried with that patch. That didn't fix this issue. Are we adding something to the browser search? Is the suggestions prompt a header?

Comment 3

[:Margaret Leibovic]

Is the problem just when long-clicking or when regular clicking?

For regular clicks, we have this adjustment:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/BrowserSearch.java#233

Probably need to add the same thing for long clicks.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

> Is the problem just when long-clicking or when regular clicking?

Just long-clicking (using 8/27's Nightly).

Comment 5

[:Margaret Leibovic]

Attachment: quick patch

This fixes the issue, but I don't know if it's the most elegant solution.

Comment 6

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 796798 [details] [diff] [review]
quick patch

Review of attachment 796798 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.

Comment 7

[:Margaret Leibovic]

Comment on attachment 796798 [details] [diff] [review]
quick patch

Upgrading to a review request. I'll get rid of that stray import line that got added to HomeListView.

Comment 8

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 796798 [details] [diff] [review]
quick patch

Review of attachment 796798 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with unused import removed.

Comment 9

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/098b935f5f3a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/098b935f5f3a

Comment 11

[Aaron Train [:aaronmt]]

Verified Fixed. Didn't even know we had a context menu on those.