1388863 - Bookmarking a link via Activity Stream context menu should give feedback

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement)

Description

[Ursula Sarracini (:ursula)]

If you've bookmarked a link via Activity Stream's context menu (the "..." that appear when hovering over a card), it should open up a bookmarking dialog to give some visual feedback that the bookmarking action went through successfully.

Preferably we want to use the shiny new Photon designs for bookmarking a page, but if that isn't possible (we would have to add some new functions to browser-places.js since this is the first instance where we show a UI for a page we're bookmarking that isn't actually loaded) then we can stick with the old showBookmarkDialog panel.

Comment 1

[Ursula Sarracini (:ursula)]

Attachment: Bug 1388863 - Bookmarking a link via Activity Stream context menu should give feedback

Review commit: https://reviewboard.mozilla.org/r/166754/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166754/

Comment 2

[Drew Willcoxon :adw]

Comment on attachment 8895536 [details]
Bug 1388863 - Bookmarking a link via Activity Stream context menu should give feedback

https://reviewboard.mozilla.org/r/166754/#review171890

This is basically what I had in mind too, but it looks like there are a few more uses of aBrowser in bookmarkPage and _bookmarkPagePT that you'll ultimately need to account for.  I think it's OK to set docInfo to {} if !aBrowser (or to null, but {} might be easier), and to leave charset null.

I think you'll need to update the !useAsyncTransactions case in bookmarkPage too because right now async transactions are only enabled on Nightly, and I'm not sure when that will change.

Comment 3

[Ursula Sarracini (:ursula)]

Comment on attachment 8895536 [details]
Bug 1388863 - Bookmarking a link via Activity Stream context menu should give feedback

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166754/diff/1-2/

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 8895536 [details]
Bug 1388863 - Bookmarking a link via Activity Stream context menu should give feedback

https://reviewboard.mozilla.org/r/166754/#review173642

Thanks Ursula, looks good.

::: toolkit/modules/NewTabUtils.jsm:1139
(Diff revision 2)
> +   * the bookmarking action has been successful
>     *
> -   * @param {String} aUrl
> -   *          The url to bookmark
> +   * @param {Object} aData
> +   *          aData.url The url to bookmark
> +   *          aData.title The title of the page to bookmark
>     *

Need a @param for aBrowser

Comment 5

[Ursula Sarracini (:ursula)]

Comment on attachment 8895536 [details]
Bug 1388863 - Bookmarking a link via Activity Stream context menu should give feedback

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166754/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44b5074b9c65
Bookmarking a link via Activity Stream context menu should give feedback r=adw

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/44b5074b9c65

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/c151548456e31251cabe02252c432a094bbf8375
chore(places): Backport bug 1388863 add bookmarking UI feedback (#3154)

Comment 9

[Cristina Badescu [:cbadescu], Experiments QA]

When bookmarking a link via Activity Stream context menu, it opens a bookmark dialog and there is a visual feedback that express the action was successfully performed. Verified as fixed using latest Nightly 57.0a1 Build ID 20170918220054, on Windows 10 x64, Mac 10.12 and Ubuntu 14.04 x64.