Closed Bug 677630 Opened 8 years ago Closed 8 years ago

Extend 'Add to Home Screen' to Bookmark Popup

Categories

(Firefox for Android Graveyard :: Bookmarks, defect)

defect
Not set

Tracking

(firefox8 fixed)

VERIFIED FIXED
Firefox 8
Tracking Status
firefox8 --- fixed

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #551841 - Flags: review?(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.)
Attachment #551841 - Flags: review?(mbrubeck) → review+
Assignee: nobody → mark.finkle
Component: General → Bookmarks
Depends on: 676293
OS: Linux → All
QA Contact: general → bookmarks
Hardware: x86 → All
(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.
http://hg.mozilla.org/mozilla-central/rev/04dfb49d3a3d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Flags: in-litmus?(kbrosnan)
Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110810 Firefox/8.0a1 Fennec/8.0a1
Status: RESOLVED → VERIFIED
Depends on: 677975
No longer depends on: 677975
Flags: in-litmus?(kbrosnan)
You need to log in before you can comment on or make changes to this bug.