1149869 - Repeatedly adding and removing an item from reader mode causes duplicate items to appear in the sidebar

Status: VERIFIED FIXED

(Firefox Graveyard :: Reading List, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0010-Bug-XXXXXXX-prevent-duplicate-readinglist-items-from.patch

STR:
* With or without syncing enabled, open a page in reader mode and repeatedly click the "+-" button.

Actual:
* Duplicate items stack up in the sidebar, which is fixed on restart.

Expected:
* No dupes.

The problem is that the sidebar only removes the node from its map on the transitionend event. If the item is re-added before this event fires it gets confused.  The fix is to remove the item from the map directly in onItemDeleted.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/fx-team/rev/3eb16083368b

The problem was introduced in bug 1147444, so blocking that - if that gets uplifted this needs to as well.

(FTR, I accidentally pushed an empty commit as https://hg.mozilla.org/releases/mozilla-aurora/rev/69327970ef09 which TomCat said he'd back out for me - needinfo him to make sure that isn't forgotten)

Comment 2

[Carsten Book [:Tomcat]]

(In reply to Mark Hammond [:markh] from comment #1)
> https://hg.mozilla.org/integration/fx-team/rev/3eb16083368b
> 
> The problem was introduced in bug 1147444, so blocking that - if that gets
> uplifted this needs to as well.
> 
> (FTR, I accidentally pushed an empty commit as
> https://hg.mozilla.org/releases/mozilla-aurora/rev/69327970ef09 which TomCat
> said he'd back out for me - needinfo him to make sure that isn't forgotten)

Hey Ryan, qbackout fails here (abort: stdin: no diffs found) so do we need to do anything here ?

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Nothing to back out of the diff is empty. What's done is done at this point.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3eb16083368b

Comment 5

[Andrei Vaida [:avaida]]

Verified fixed on Nightly 40.0a1 (2015-04-02), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

(In reply to Mark Hammond [:markh] from comment #1)
> The problem was introduced in bug 1147444, so blocking that - if that gets
> uplifted this needs to as well.

Fat log of good that did me - that bug got uplifted :(

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8586534 [details] [diff] [review]
0010-Bug-XXXXXXX-prevent-duplicate-readinglist-items-from.patch

Approval Request Comment
[Feature/regressing bug #]: readinglist
[User impact if declined]: Items may be added multiple times
[Describe test coverage new/current, TreeHerder]: Already landed on central
[Risks and why]: None
[String/UUID change made/needed]: None

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8586534 [details] [diff] [review]
0010-Bug-XXXXXXX-prevent-duplicate-readinglist-items-from.patch

Should be in 38 beta 3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a6f8ea157868

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/881a59941b04

Comment 12

[Andrei Vaida [:avaida]]

Verified fixed on 38.0b3-build1 (20150409144858) as well, using Ubuntu 14.04 (x64), Windows 8.1 (x64) and Mac OS X 10.9.5.

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing qe-verify flag as it was verified on Firefox 38 Beta and Firefox 40 Nightly.