Closed Bug 1311555 Opened 3 years ago Closed 3 years ago

Consider filling bookmark star in AS context menu for bookmarked pages

Categories

(Firefox for Android :: Awesomescreen, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 53
Iteration:
1.13
Tracking Status
firefox53 --- fixed

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

(Whiteboard: [MobileAS])

Attachments

(2 files)

The Activity Stream context menu has a bookmark item: the label for this menu item changes depending on bookmark star, however the icon is a static hollow star.

Do we want to show a filled (blue) star for consistency with the main app menu?
Assignee: nobody → ahunt
Priority: -- → P1
Summary: Consideri filling bookmark star in AS context menu for bookmarked pages → Consider filling bookmark star in AS context menu for bookmarked pages
Whiteboard: [MobileAS]
Depends on: 1300144
Iteration: --- → 1.7
Iteration: 1.7 → ---
Priority: P1 → P2
Assignee: ahunt → nobody
Assignee: nobody → ahunt
Iteration: --- → 1.13
Priority: P2 → P1
Status: NEW → ASSIGNED
Comment on attachment 8827853 [details]
Bug 1311555 - Use filled bookmark star in ActivityStream context menu

https://reviewboard.mozilla.org/r/105452/#review106656
Attachment #8827853 - Flags: review?(s.kaspari) → review+
Comment on attachment 8827852 [details]
Bug 1311555 - Pre: remove unused imports

https://reviewboard.mozilla.org/r/105450/#review106762
Attachment #8827852 - Flags: review+
Backed out for Android build bustage:

https://hg.mozilla.org/integration/autoland/rev/4a24f1c6fd662b9629952a086ae9ead1d532d6be
https://hg.mozilla.org/integration/autoland/rev/f3bdcf57e1cfd674cc145edc5f9578e9293fc84c

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d8c85b0ed424e7e92c6b4ba30f756a0f5af04174
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=70407113&repo=autoland

[task 2017-01-19T17:55:25.488011Z] 17:55:25     INFO -    Copying resources from program jar [/home/worker/workspace/build/src/obj-firefox/dist/exploded-aar/play-services-measurement-8.4.0/play-services-measurement-8.4.0-classes.jar]
[task 2017-01-19T17:55:27.654911Z] 17:55:27     INFO -  /home/worker/workspace/build/src/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java:83: error: cannot find symbol
[task 2017-01-19T17:55:27.655000Z] 17:55:27     INFO -              bookmarkItem.setIcon(R.drawable.as_bookmark_filled);
[task 2017-01-19T17:55:27.655048Z] 17:55:27     INFO -                                             ^
[task 2017-01-19T17:55:27.656964Z] 17:55:27     INFO -    symbol:   variable as_bookmark_filled
[task 2017-01-19T17:55:27.657021Z] 17:55:27     INFO -    location: class drawable
[task 2017-01-19T17:55:27.657107Z] 17:55:27     INFO -  /home/worker/workspace/build/src/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java:113: error: cannot find symbol
[task 2017-01-19T17:55:27.657170Z] 17:55:27     INFO -                          bookmarkItem.setIcon(R.drawable.as_bookmark_filled);
[task 2017-01-19T17:55:27.657223Z] 17:55:27     INFO -                                                         ^
[task 2017-01-19T17:55:27.657268Z] 17:55:27     INFO -    symbol:   variable as_bookmark_filled
[task 2017-01-19T17:55:27.657308Z] 17:55:27     INFO -    location: class drawable
[task 2017-01-19T17:55:28.416748Z] 17:55:28     INFO -  Note: Some input files use or override a deprecated API.
[task 2017-01-19T17:55:28.416953Z] 17:55:28     INFO -  Note: Recompile with -Xlint:deprecation for details.
[task 2017-01-19T17:55:28.417089Z] 17:55:28     INFO -  2 errors
Flags: needinfo?(ahunt)
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57ccc83a450f
Pre: remove unused imports r=ahunt
I majorly messed this up - the new VectorDrawable (when added by Android Studio's Vector Studio), ended up in a different resource directory - mach builds ignore that location, and I didn't do a try build to discover that either...
Flags: needinfo?(ahunt)
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f560f0eef6be
Pre: remove unused imports r=ahunt
https://hg.mozilla.org/integration/autoland/rev/ef92919b4aca
Use filled bookmark star in ActivityStream context menu r=sebastian
https://hg.mozilla.org/mozilla-central/rev/f560f0eef6be
https://hg.mozilla.org/mozilla-central/rev/ef92919b4aca
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Tested with the latest Nightly build 54.0a1 (2017-02-16) using the following devices:
- Huawei MediaPad M2 (Android 5.1.1);
- Asus Transformer Pad (Android 4.2.1);
- Samsung Galaxy Note 4 (Android 5.0.1).

Bookmark star icon is now filled when adding a website to Bookmarks (for both Top sites and Highlights context menus).

I'm marking this as Verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.