515995 - Vertical panning possible when results are less than available screen size

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Aakash Desai [:aakashd]]

Build Id:

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.2a2pre) Gecko/20090910
Fennec/1.0b4pre

Steps to Reproduce:
1. Click on the url bar to start the awesome bar
2. Start a search that will populate a set of results that is less than the available vertical list size
3. Begin panning down

Actual Results:
We can pan down infinitely

Expected Results:
Panning shouldn't be allowed when there isn't a need for it.

Comment 1

[Aakash Desai [:aakashd]]

This happens on today's nightly on WinMo as well:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a2pre) Gecko/20090911
Fennec/1.0a3

Comment 2

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

> Actual Results:
> We can pan down infinitely
This is probably related to the fact that we initially populated 12 rows for perfs reasons (so it's not infinitely :) )
> 
> Expected Results:
> Panning shouldn't be allowed when there isn't a need for it.

right.

Comment 3

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

Attachment: Patch

I'm wondering if this can slow down the awesome bar cause of reflow?

Comment 4

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

Attachment: Patch v0.2

For the explanation of the rule see:
 * https://developer.mozilla.org/en/CSS/:nth-last-child#Example_selectors

Comment 5

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

hmm, maybe it's easier to read to do the contrary:
 * turns all the rows with value="" to display:none
 * turns the first 2 row with value="" to visibility: hidden and revert back the display

This way if we add more rows we don't need to tweak the css.

Comment 6

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

Attachment: Patch

ok. I think that's the right way to do it

Comment 7

[:Gavin Sharp [email: [e-mail]]]

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

>diff -r a22f1e986752 chrome/content/browser.css

>-autocompleteresult[value=""] {
>+autocompleteresult[value=""]:first-child {
>   visibility: hidden;
> }
> 
>+autocompleteresult[value=""]:not(:first-child) {
>+  display: none;
>+}

So the reason we want the first-child to always be visible is that we depend on it having a height for the "no results" item to be properly positioned, right?

r=me with a comment added to make that clear, assuming it's correct :)

Comment 8

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

Attachment: Patch

(In reply to comment #7)

> So the reason we want the first-child to always be visible is that we depend on
> it having a height for the "no results" item to be properly positioned, right?
> 
Correct :)

Comment 9

[:Gavin Sharp [email: [e-mail]]]

pushed, with a comment tweak:
https://hg.mozilla.org/mobile-browser/rev/347014dd06f0

Comment 10

[Aakash Desai [:aakashd]]

verified FIXED On builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a2pre) Gecko/20090923 Fennec/1.0a3

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.2a2pre) Gecko/20090923
Fennec/1.0b4pre

Comment 11

[Aakash Desai [:aakashd]]

litmus testcase 9801 has been added to regression test this bug.