1433492 - RSS feeds don't remember their read/unread status after a restart

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Mark Banner (:standard8)]

The changes in Bug 1370881 caused a minor regression whereby viewing an RSS feed (e.g. on the toolbar) after startup would not have indications of previously read articles.

STR:

1) Add a feed to the bookmarks toolbar.
2) Open the feed, read an article or two.
3) Restart Firefox.
4) Open the feed again

Actual Result

=> Feed items are not marked as read

Expected Result

=> The feed items that were read are now marked as read again.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1433492 - Fix a missed callback to .then conversion causing RSS feeds to not show their read/unread status properly after a restart.

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

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1433492 - Remove remaining instances of PlacesUtils.asyncHistory.isURIVisited being called directly.

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

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8945828 [details]
Bug 1433492 - Remove remaining instances of PlacesUtils.asyncHistory.isURIVisited being called directly.

https://reviewboard.mozilla.org/r/215932/#review221734

::: browser/base/content/test/general/head.js:314
(Diff revision 1)
>      let callbackCount = 0;
>      let niceStr = aShouldBeCleared ? "no longer" : "still";
>      function callbackDone() {
>        if (++callbackCount == aURIs.length)
>          resolve();
>      }

what about making promiseHistoryClearedState an async function, removing the crazy callbackDone thing and just use a for...of loop?

for (let uri of aURIs) {
  let visited = await PlacesUtils.history.hasVisits...
  Assert.equal(isVisited, !aShouldBeCleared, ...);
}

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8945827 [details]
Bug 1433492 - Fix a missed callback to .then conversion causing RSS feeds to not show their read/unread status properly after a restart.

https://reviewboard.mozilla.org/r/215914/#review221728

::: toolkit/components/places/nsLivemarkService.js:606
(Diff revision 1)
>  
>      // Update visited status for each entry.
>      for (let child of this._children) {
> -      history.hasVisits(child.uri, isVisited => {
> +      history.hasVisits(child.uri).then(isVisited => {
>          this.updateURIVisitedStatus(child.uri, isVisited);
>        });

probably this should also .catch(Cu.reportError);

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8945827 [details]
Bug 1433492 - Fix a missed callback to .then conversion causing RSS feeds to not show their read/unread status properly after a restart.

https://reviewboard.mozilla.org/r/215914/#review221736

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8945827 [details]
Bug 1433492 - Fix a missed callback to .then conversion causing RSS feeds to not show their read/unread status properly after a restart.

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

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8945828 [details]
Bug 1433492 - Remove remaining instances of PlacesUtils.asyncHistory.isURIVisited being called directly.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee3ef79bf57b
Fix a missed callback to .then conversion causing RSS feeds to not show their read/unread status properly after a restart. r=mak
https://hg.mozilla.org/integration/autoland/rev/f1f8869f631a
Remove remaining instances of PlacesUtils.asyncHistory.isURIVisited being called directly. r=mak

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ee3ef79bf57b
https://hg.mozilla.org/mozilla-central/rev/f1f8869f631a

Comment 10

[David Olah]

I verified on Windows 10 x64 on Nighly 59.0a1 (2018-01-01) [before fix] and on Nightly 60.0a1 (2018-03-12) [last version, after fix] and didn't managed to reproduce on neither of them.

Is this an OS specific bug or it can be marked as verified if I retested on Windows 10?

Comment 11

[Mark Banner (:standard8)]

This was a cross platform issue.

Comment 12

[David Olah]

Verified on Nightly 60.0a1 (2018-03-12) on:
- Windows 10 x64
- Windows 7 Professional x64
- Ubuntu 16.04
- Mac OS

Based on Comment 10 and Comment 11, I mark this issue as Verified.