532766 - tapping "manage" in bookmark list scrolls you to the very first bookmark

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Bookmarks, defect)

Description

[Madhava Enros [:madhava]]

Scenario:

- pan down, say, three pages worth of bookmarks to the one you want to manage
- tap manage
- manager has scrolled all the way up to the first bookmark and pops it open for editing (at which point you _can_ close that one and pan down to the one you wanted, but it's annoying)

The intended design was that, when you tap "manage", it pops open the first one visible on the screen (currently) to make it clear what managing does.  I thought that Fennec was actually doing this as I've described at one point, but I could be mistaken.

Comment 1

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

Attachment: Patch

Edit the first _fully_ visible row

Comment 2

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

Mark, I've looked at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/richlistbox.xml#282 as you suggested but this didn't work because the children property of the richlistbox returns 0 in our case (because of http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/richlistbox.xml#357).

And other point I think what we want is to edit the first completely visible rows and _itemVisible returns the row even if it's partially visible. If needed I can used this one and took the next sibling if the row is partially visible.

But maybe i'm wrong and madhava's wants tan other behavior (aka editing a row even if it's not fully visible - after testing both it's not what i think is useful)

Comment 3

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

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

Let's change _editFirstVisibleRow to _getFirstVisibleRow and then we can use it as a utility method.

I can do that when landing

Comment 4

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

pushed:
https://hg.mozilla.org/mobile-browser/rev/8a1a0a37d614

Comment 5

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b5pre) Gecko/20091211 Firefox/3.6b5pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091211 Firefox/3.7a1pre Fennec/1.0b5

Comment 6

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

(In reply to comment #2)
> as you suggested but this didn't work because the children property of the
> richlistbox returns 0 in our case (because of
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/richlistbox.xml#357).

This seems like something we should fix. Is this happening because the richlistbox's children aren't wrapped in richlistitems?