Closed Bug 810419 Opened 7 years ago Closed 7 years ago
Duplicate Tools > Social Provider ambient-menuitems are not removed
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.
I assume this affects Firefox 17 through 19?
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.
I would argue wontfix for Firefox 17 given how close we are to release, unless you feel it is critical.
OS: Windows 7 → All
Hardware: x86_64 → All
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
Attachment #680180 - Flags: review?(felipc)
Attachment #680180 - Flags: review?(felipc) → review+
followup to fix the test description, https://hg.mozilla.org/integration/mozilla-inbound/rev/4214cae0c9bb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
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.
Attachment #680180 - Flags: approval-mozilla-aurora?
Attachment #680180 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.
You need to log in before you can comment on or make changes to this bug.