810419 - Duplicate Tools > Social Provider ambient-menuitems are not removed

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

After bug 808213 got fixed, I noticed some duplicate menuitem entries in the keyboard-access Tools > Facebook Messenger menu.

I added some logging statements to the code and saw this:

Starting to populate
Previously had 0 ambient-menuitems
Now after removal, there are 0 ambient-menuitems left
Adding friends-jewel
Finished populating

Starting to populate
Previously had 1 ambient-menuitems
Removing See All Friend Requests
Now after removal, there are 0 ambient-menuitems left
Adding friends-jewel
Adding messages-jewel
Finished populating

Starting to populate
Previously had 2 ambient-menuitems
removing See All Friend Requests
Now after removal, there are 1 ambient-menuitems left
Adding friends-jewel
Adding messages-jewel
Adding notifications-jewel
Finished populating

I'm not sure why the second menuitem wasn't removed in that third pass through of updates.

Comment 1

[u279076]

I assume this affects Firefox 17 through 19?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Yes, it appears so.

I've narrowed this down to a bug in the JS engine's implementation of for-of loops and filed bug 810438.

Comment 3

[u279076]

I would argue wontfix for Firefox 17 given how close we are to release, unless you feel it is critical.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

This works around the JS bug by using a different approach to clear the menu.

Pushed to tryserver, https://tbpl.mozilla.org/?tree=Try&rev=8416e60ae941

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8bc8bb4b007

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

followup to fix the test description, https://hg.mozilla.org/integration/mozilla-inbound/rev/4214cae0c9bb

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8bc8bb4b007
https://hg.mozilla.org/mozilla-central/rev/4214cae0c9bb

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 680180 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 801040
User impact if declined: duplicate menuitems in the social menu
Testing completed (on m-c, etc.): m-c and in testsuite
Risk to taking this patch (and alternatives if risky): none expected, trivial change
String or UUID changes made by this patch: none

Assuming wontfix for Fx17 due to the timing.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/83c532f6b277

Comment 10

[u279076]

Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.