Closed Bug 515995 Opened 10 years ago Closed 10 years ago

Vertical panning possible when results are less than available screen size

Categories

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

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
fennec1.0b4

People

(Reporter: aakashd, Assigned: vingtetun)

Details

Attachments

(2 files, 2 obsolete files)

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.
tracking-fennec: --- → ?
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
OS: Linux (embedded) → All
Hardware: ARM → All
> 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.
Attached patch Patch (obsolete) — Splinter Review
I'm wondering if this can slow down the awesome bar cause of reflow?
Attachment #400277 - Flags: review?(gavin.sharp)
Attached patch Patch v0.2 (obsolete) — Splinter Review
For the explanation of the rule see:
 * https://developer.mozilla.org/en/CSS/:nth-last-child#Example_selectors
Attachment #400277 - Attachment is obsolete: true
Attachment #401864 - Flags: review?(gavin.sharp)
Attachment #400277 - Flags: review?(gavin.sharp)
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.
Attached patch PatchSplinter Review
ok. I think that's the right way to do it
Attachment #401864 - Attachment is obsolete: true
Attachment #402088 - Flags: review?(gavin.sharp)
Attachment #401864 - Flags: review?(gavin.sharp)
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 :)
Attachment #402088 - Flags: review?(gavin.sharp) → review+
Attached patch PatchSplinter Review
(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 :)
pushed, with a comment tweak:
https://hg.mozilla.org/mobile-browser/rev/347014dd06f0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → B4
Component: Linux/Maemo → General
QA Contact: maemo-linux → general
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
Status: RESOLVED → VERIFIED
Flags: in-litmus?
litmus testcase 9801 has been added to regression test this bug.
Flags: in-litmus? → in-litmus+
Component: General → Panning/Zooming
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.