494191 - move bookmark button out of titlebar

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Bookmarks, defect)

Description

[Madhava Enros [:madhava]]

Basically, on smaller screens and in portrait mode even in larger screens, we just don't have much room in the titlebar.  The bookmarks button can probably be moved without harming functionality.  This blog post sums up the problem, and my suggested new placement for access to bookmarks:

http://madhava.com/egotism/archive/005033.html

In other words, making the first row in the pre-typing awesomebar set be a "[icon] see all bookmarks" entry.  Tapping on it takes you to the boomarks list, in its already existing form.

Comment 2

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

Attachment: WIP

Item doesn't work yet (can't select it), and styling isn't quite right (also need corresponding wince theme changes).

Comment 3

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

Attachment: patch

Styling is pretty basic, and had to jump through some hoops to get the right selection behavior without affecting what the autocomplete controller saw, but otherwise seems to work pretty well.

Comment 4

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

Attachment: patch v2

Only one change since the last patch ("hack" comment in the first hunk), to fix a problem Madhava found: when backspacing to remove all text, we should revert to the "All results" popup.

Comment 5

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

Comment on attachment 391421 [details] [diff] [review]
patch v2

This looks good so far. I know you have another patch coming with slight changes too.

Comment 6

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

Attachment: patch+styling

Adds styling as discussed. Haven't been able to test the WinCE themes, help there would be appreciated. bookmarks-24.png is currently just a copy of bookmarks-30.png, will need to update it when I get the resized image from Sean.

Comment 7

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

Attachment: interdiff for review

interdiff of attachment 391421 [details] [diff] [review] (r+ed) and attachment 391817 [details] [diff] [review]

Comment 8

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

Comment on attachment 391818 [details] [diff] [review]
interdiff for review

nit: rename arrowRight-16.png to arrowright-16.png

Comment 9

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

Comment on attachment 391817 [details] [diff] [review]
patch+styling

r+ based on the first patch and the interdiff, with nits of the interdiff

Comment 10

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

https://hg.mozilla.org/mobile-browser/rev/3f3369d81a88

Might require some style tweaks on Wince, will do that in followups.

Comment 11

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

Attachment: patch (winmo tweaks)

* Minor tweaks for WinMo
* arrowRight-16 -> arrowright-16 name change in CSS

Screenshot to follow

Comment 12

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

Screenshot of the tweaks:
http://people.mozilla.com/~mfinkle/fennec/screenshots/fennec-bookmark-awesomebar.png

Comment 13

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

pushed wince changes: http://hg.mozilla.org/mobile-browser/rev/ae70777017a3

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on builds:


Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090817
Fennec/1.0b3pre

and

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090817 Fennec/1.0a3pre

Comment 15

[Aakash Desai [:aakashd]]

litmus testcases:


https://litmus.mozilla.org/show_test.cgi?id=7097 
https://litmus.mozilla.org/show_test.cgi?id=9848
https://litmus.mozilla.org/show_test.cgi?id=9789

...are in litmus to regression test this bug.