779497 - The Add-ons Manager entries are multiplied if a search engine is added from context menu

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Add-on Manager, defect)

Description

[Paul Feher]

Firefox Mobile Native 15.0b3 build1 (2012-07-31)
Device: HTC Desire Z(Android 2.3.3)/Samsung Galaxy R (Android 2.3.4)

Steps to reproduce:
1. Install any add-on(ex. cloud viewer).
2. Open Add-ons Manager.
3. In another tab visit www.cnet.com.
4. Long-tap in the search field to bring up the Context Menu. Choose "Add Search Engine".
5. Switch tab to Add-ons Manager.
6. Repeat steps 3 to 5.

Expected results:
After step 5 the list of search engines is displayed no entries are multiplied.

Actual results:
After step 6 the list of search engines is displayed and all entries appear multiplied.

Comment 1

[Aaron Train [:aaronmt]]

(In reply to Paul Feher from comment #0)
> 1. Visit www.google.com
> 2. Long-tap in the search field to bring up the Context Menu. Choose "Add
> Search Engine".
> 3. Repeat steps 1 and 2.

Fixed.

Comment 2

[Paul Feher]

(In reply to Aaron Train [:aaronmt] from comment #1)
> (In reply to Paul Feher from comment #0)
> > 1. Visit www.google.com
> > 2. Long-tap in the search field to bring up the Context Menu. Choose "Add
> > Search Engine".
> > 3. Repeat steps 1 and 2.
> 
> Fixed.

This issue refers to the fact that after installing a search engine from context menu all the entries that were previously present in the Add-ons manager list will be multiplied by 3. Please see the video for clearer steps to reproduce.
  
http://youtu.be/IL4tbHi_L8A

Comment 3

[Paul Feher]

For the the other issue, regarding the fact that a search engine can be added multiple times from the context menu I've logged the issue: Bug 779739

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

This disappears on restarting Firefox. Trunk is still affected by this issue.

Comment 5

[Ioana Chiorean]

The multiplications disappear if one default search engine is disabled. 

Build ID: 17.0a2 (2012-08-29) Aurora Channel
Device: Galaxy Nexus 
OS: Android 4.0.4

Comment 6

[Adrian Tamas (:AdrianT)]

This is still an issue on Aurora 23.0a2 2013-05-15 on the HTC Desire Z (Android 2.3.3)

Comment 7

[Brian Nicholson (:bnicholson)]

Attachment: Clear addons list inside of async callback

When adding a search engine, callbacks are fired three times: "engine-changed" is fired twice, then "engine-added" is fired after that. I haven't looked into why "engine-changed" is fired twice, or if we even "engine-added"/"engine-removed" if we already listen for "engine-changed", but I just left it alone for now.

The problem is that AddonManager.getAddonsByTypes() is asynchronous, and we clear the addons list before the async callback happens. So the list is cleared three times, then the callbacks come in and the list is populated three times. We can fix this by moving the logic to clear the list inside of the callback.

Comment 8

[Wesley Johnston (:wesj)]

Comment on attachment 751219 [details] [diff] [review]
Clear addons list inside of async callback

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

Not my favorite patch. How hard would it be to find the right Addon item and just fix/update it?

Comment 9

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #8)
> Comment on attachment 751219 [details] [diff] [review]
> Clear addons list inside of async callback
> 
> Review of attachment 751219 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Not my favorite patch. How hard would it be to find the right Addon item and
> just fix/update it?

It would take a bit of refactoring, but definitely doable. But since this is the easy fix for the existing code that requires minimal time/changes, I'd prefer to just land this now, and we can handle optimizing things in a separate bug.

Comment 10

[Brian Nicholson (:bnicholson)]

Filed bug 878968.

Comment 11

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c5ff4fdd7d8

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 24.0a1 (2013-06-04)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1