Closed
Bug 1184005
Opened 9 years ago
Closed 9 years ago
Remove ReadingList from the tree.
Categories
(Firefox Graveyard :: Reading List, defect)
Firefox Graveyard
Reading List
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 42
People
(Reporter: markh, Assigned: markh)
References
Details
Attachments
(4 files)
(deleted),
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
MattN
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
Over in bug 1180587 we are removing all Sync "error bars" from the browser window. The only surfaced Sync error will be "needs reauthentication" and that will be surfaced in the hamburger menu. ReadingList uses the infobar, so is almost certainly going to be broken by this change. It seems unlikely ReadingList is going to come back. Should we just kill it? We can always get it back from hg.
Comment 1•9 years ago
|
||
(In reply to Mark Hammond [:markh] from comment #0)
> It seems unlikely
> ReadingList is going to come back. Should we just kill it? We can always get
> it back from hg.
I agree. Pocket has been successful at providing this class of feature, and so I think it's very unlikely we'd spend all the time and resources to get Reading List (and Sync) fully finished. Especially because we'd then have 2 almost-but-not-quite-the-same services, and that would be confusing for users.
That said, I don't feel like we ever can to closure on what to do with the Reading List feature that Android has long shipped. I guess ideally, presuming the above, that it should migrate to / be replaced by Pocket?
mfinkle might have an opinion here!
Flags: needinfo?(mark.finkle)
Comment 2•9 years ago
|
||
It's looking more and more like Reading List, across platforms, will never happen.
I understand wanting to keep the shipping code clean and maintainable, so I think you should move ahead with removing the Desktop RL code from the tree.
Mobile has more work to do if we decide to drop RL. We'll need to discuss how to migrate data. That should not block Desktop.
Flags: needinfo?(mark.finkle)
Assignee | ||
Comment 3•9 years ago
|
||
I've split these patches somewhat arbitrarily to help share the review lovin'.
This patch removes browser/components/readinglist. Drew, you helped land alot of this, so helping to unland it seems a reasonable next step :)
Assignee | ||
Comment 4•9 years ago
|
||
Jaws, are you able to have a look at the styling changes?
Attachment #8637596 -
Flags: feedback?(jaws)
Assignee | ||
Comment 5•9 years ago
|
||
Matt, these are the core UI components.
Attachment #8637597 -
Flags: feedback?(MattN+bmo)
Assignee | ||
Comment 6•9 years ago
|
||
Irakli, there are some references to the readinglist sidebar in the addon-sdk code. Could you have a quick look at this, or suggest someone else who could have a look?
Attachment #8637598 -
Flags: feedback?(rFobic)
Assignee | ||
Comment 7•9 years ago
|
||
Try run with these patches at https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea437b884a0c
Comment 8•9 years ago
|
||
Comment on attachment 8637596 [details] [diff] [review]
0002-Bug-1184005-Remove-readinglist-part-x-remove-styling.patch
Review of attachment 8637596 [details] [diff] [review]:
-----------------------------------------------------------------
rs=me
Attachment #8637596 -
Flags: feedback?(jaws) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8637595 [details] [diff] [review]
0001-Bug-1184005-Remove-readinglist-part-x-remove-browser.patch
Review of attachment 8637595 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine.
Attachment #8637595 -
Flags: feedback?(adw) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8637597 [details] [diff] [review]
0003-Bug-1184005-Remove-readinglist-part-x-remove-UI-and-.patch
Review of attachment 8637597 [details] [diff] [review]:
-----------------------------------------------------------------
rs=me
Attachment #8637597 -
Flags: feedback?(MattN+bmo) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8637598 [details] [diff] [review]
0004-Bug-1184005-Remove-readinglist-part-x-addon-sdk-part.patch
Drew, you are also listed as a peer for the addon-sdk and I think I've waited long enough for :gozala - the patch is quite simple.
Attachment #8637598 -
Flags: feedback?(rFobic) → review?(adw)
Updated•9 years ago
|
Attachment #8637598 -
Flags: review?(adw) → review+
Comment 13•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment 14•9 years ago
|
||
Commit pushed to master at https://github.com/mozilla/addon-sdk
https://github.com/mozilla/addon-sdk/commit/153b38625706f8e2abe7a457cf2bb4b7757b398c
Bug 1184005 - Remove readinglist. r=MattN,jaws,adw
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
Updated•8 years ago
|
status-firefox42:
fixed → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•