Closed Bug 1144823 Opened 5 years ago Closed 5 years ago

reading list sidebar should be sorted by most recently added

Categories

(Firefox Graveyard :: Reading List, defect)

defect
Not set

Tracking

(firefox38 verified, firefox39 verified)

VERIFIED FIXED
Firefox 39
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: clarkbw, Assigned: florian)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

currently the most recently added items are appending to the list, we want those items pre-pended and that sort order to be constant.
Flags: qe-verify+
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attachment #8580426 - Flags: review?(mhammond)
Hi Florian, can you provide a point value.
Iteration: --- → 39.2 - 23 Mar
Flags: needinfo?(florian)
Flags: firefox-backlog+
Points: --- → 2
Flags: needinfo?(florian)
Blocks: 1132074
Comment on attachment 8580426 [details] [diff] [review]
Patch

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

As we discussed, let's just get ReadingList.jsm to populate the added-on date and use this as the sort key.
Attachment #8580426 - Flags: review?(mhammond)
QA Contact: andrei.vaida
Attached patch Patch v2Splinter Review
As discussed, this populates the addedOn field of the database, and also sorts the bookmarks popup.
Attachment #8580426 - Attachment is obsolete: true
Attachment #8580826 - Flags: review?(mhammond)
Comment on attachment 8580826 [details] [diff] [review]
Patch v2

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

::: browser/components/readinglist/sidebar.js
@@ +90,5 @@
>      log.trace(`onItemAdded: ${item}`);
>  
>      let itemNode = document.importNode(this.itemTemplate.content, true).firstElementChild;
>      this.updateItem(item, itemNode);
> +    if (append)

Please add a comment indicating this is a temp hack and will need to be re-worked when items come from the sync engine.
Attachment #8580826 - Flags: review?(mhammond) → review+
This broke test_ReadingList.js, but I landed a fix in 6305033b955e in bug 1131416.
https://hg.mozilla.org/mozilla-central/rev/b11aaab4a248
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39
Verified fixed on Nightly 39.0a1 (2015-03-23), using Ubuntu 14.04 (x64), Mac OS X 10.9.5 and Windows 7 (x64).
Status: RESOLVED → 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.
(In reply to Mark Hammond [:markh] from comment #5)

> Please add a comment indicating this is a temp hack and will need to be
> re-worked when items come from the sync engine.

Filed bug 1149823 for this.
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.