[ReadingList] Open the Reading List sidebar when the button in the location bar is used to add the current page to Reading List

VERIFIED FIXED in Firefox 38

Status

()

Toolkit
Reader Mode
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Unfocused, Assigned: jaws)

Tracking

(Blocks: 1 bug)

unspecified
mozilla39
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox38 verified, firefox39 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

The ReaderMode controls have a "Add to Reading List" button - on desktop, this doesn't currently do anything. It probably should.

For v1, it should just add the current page. Bug  1133429 will provide an API to pass a tab/browser to (and it'll handle all the metadata stuff).
Flags: qe-verify+
Flags: firefox-backlog+
Depends on: 1133429

Updated

3 years ago
Blocks: 1132074
I think we should also have this button open the Reading List sidebar the first time it's used. That would help with making it obvious that it's related to Reading List, that clicking it adds an item, and generally help introduce users to the feature. This is especially true for pages that don't work in Reader View, and so there's no book icon next to this button (i.e., the user may wonder why they have a mysterious (+) button in their URL bar).
No longer blocks: 1124011
Priority: -- → P2

Updated

3 years ago
No longer blocks: 1132074
Deprioritized during backlog review.  Removed from IT 39.2 priority backlog and not part of the initial Q2 campaign release.
Note that bug 1142217 landed basic support for the +- button, but I'm not resolving as a dupe as it didn't address automatically opening the sidebar.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Summary: Hook up "Add to ReadingList" button in desktop ReaderMode → [ReadingList] Open the Reading List sidebar when the button in the location bar is used to add the current page to Reading List
Created attachment 8580390 [details] [diff] [review]
Patch
Attachment #8580390 - Flags: review?(mhammond)

Updated

3 years ago
Attachment #8580390 - Flags: review?(mhammond) → review+

Updated

3 years ago
Iteration: --- → 39.2 - 23 Mar

Updated

3 years ago
Blocks: 1132074
Keywords: checkin-needed
Jared, are you sure the behavior implemented by this patch is wanted?

Comment 1 said "open the Reading List sidebar the first time it's used".

Yesterday I remember hearing mmaslaney say we should be opening the sidebar automatically when adding an item, except maybe for the first time experience.
Flags: needinfo?(jaws)
(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> Jared, are you sure the behavior implemented by this patch is wanted?
> 
> Comment 1 said "open the Reading List sidebar the first time it's used".
> 
> Yesterday I remember hearing mmaslaney say we should be opening the sidebar
> automatically when adding an item, except maybe for the first time
> experience.

Oh, somehow I missed comment 1. I'll change the patch to only open it the first time it's used. (We discussed this in person and decided that a uitour/fte that opens the sidebar can count as that first opening since it will be doing the work of educating the user about the feature and how they are connected).
Flags: needinfo?(jaws)
Keywords: checkin-needed
QA Contact: andrei.vaida
Created attachment 8580916 [details] [diff] [review]
Patch v2

This patch only opens it the if the sidebar hasn't been opened before. Note that it needs to be rebased after some of the fallout from bug 1131416 is fixed, so I based this on top of a backout of bug 1131416.
Attachment #8580390 - Attachment is obsolete: true
Attachment #8580916 - Flags: review?(mhammond)
Comment on attachment 8580916 [details] [diff] [review]
Patch v2

Review of attachment 8580916 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/profile/firefox.js
@@ +1880,5 @@
>  #endif
>  
>  // Disable ReadingList browser UI by default.
>  pref("browser.readinglist.enabled", false);
> +pref("browser.readinglist.sidebarOpened", false);

sidebarOpened sounds a little like it should go back to false when it is closed - sidebarEverOpened? But I'm not too bothered
Attachment #8580916 - Flags: review?(mhammond) → review+
Landed with the pref renamed to sidebarEverOpened,
https://hg.mozilla.org/integration/fx-team/rev/eca6911ffb55
https://hg.mozilla.org/mozilla-central/rev/eca6911ffb55
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Verified fixed on Nightly 39.0a1 (2015-03-23), using Ubuntu 14.04 (x64), Mac OS X 10.9.5 and Windows 7 (x64). 

If the sidebar has never been enabled before, adding a web page to Reading List now automatically opens the sidebar and sets browser.readinglist.sidebarEverOpened to true.
Status: RESOLVED → VERIFIED
status-firefox39: fixed → verified
Verified fixed on Aurora 38.0a2 (2015-03-29) as well, using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.
status-firefox38: fixed → verified
Duplicate of this bug: 1147484
You need to log in before you can comment on or make changes to this bug.