677630 - Extend 'Add to Home Screen' to Bookmark Popup

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Bookmarks, defect)

Description

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

Attachment: patch

Bug 676293 added basic support to the Bookmark List context menu. This bug adds it to the Bookmark Popup as well. Based on design here:

http://cl.ly/1P3z27012u3Y00140C1U

The look should mimic the existing context menus.

The patch converts the popup to a contextmenu-ish look, while retaining the arrowbox. It also moves the code to make a bookmark shortcut to BookmarkHelper so it can be easier to reuse.

Comment 1

[Matt Brubeck (:mbrubeck)]

Comment on attachment 551841 [details] [diff] [review]
patch

>+<!ENTITY bookmarkShortcut.label    "Add to Home Screen">

Should this be sentence-case ("Add to home screen")?

>+++ b/mobile/themes/core/gingerbread/browser.css
> #share-title,
>+#bookmark-popup-title,
> #context-hint {
>   color: @color_text_default@;
>   font-size: @font_small@;
>   padding: @padding_small@;
> }

Do we need a similar change in the Froyo theme?  (I saw you and Ian talking on IRC about some additional style tweaks here.)

Comment 2

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

(In reply to Matt Brubeck (:mbrubeck) from comment #1)

> >+<!ENTITY bookmarkShortcut.label    "Add to Home Screen">
>
> Should this be sentence-case ("Add to home screen")?

Nope. Button and Menu labels are title case

> >+++ b/mobile/themes/core/gingerbread/browser.css
> > #share-title,
> >+#bookmark-popup-title,
> > #context-hint {
> >   color: @color_text_default@;
> >   font-size: @font_small@;
> >   padding: @padding_small@;
> > }
> 
> Do we need a similar change in the Froyo theme?  (I saw you and Ian talking
> on IRC about some additional style tweaks here.)

Yep, I added it, tested and showed Ian.

Comment 3

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

http://hg.mozilla.org/mozilla-central/rev/04dfb49d3a3d

Comment 4

[Aaron Train [:aaronmt]]

Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110810 Firefox/8.0a1 Fennec/8.0a1