For search, AMO sends addons that it thinks are compatible. We also do some checking client side, but those checks are redundant because of the server-side checked.
So we need to tell AMO how the client is determining compatibility, much like we do with the update pings.
Created attachment 578502 [details] [diff] [review]
This has the unfortunate side-effect of breaking AMO searches until bug 706385 is fixed, so I don't think this can be landed until that's fixed and goes live :\
Bug 707359 will make AMO not return a 404, so will allow this to land before the option is fully implemented in bug 706385.
Bug 707359 is live on the production site, so this was able to land a couple of days ago, but got held up by the tree closures.
Comment on attachment 578502 [details] [diff] [review]
Simple patch (most of it is tests), low risk. Without this, the AMO search in the Add-ons Manager won't show all compatible matching addons. It is still waiting on the AMO-side bug (bug 706385), but better to have this baking in the tree as soon as possible (which bug 707359 has allowed).
Is this something QA can verify?
Only indirectly, since we're waiting on a change to AMO's search API behaviour (bug 706385). Once that bug is fixed and goes live on the production site, the search results will change based on what compatibility mode is enabled. I'd rather not have this marked as verified until it can be tested together with the AMO change.
The unit test covers ensuring the requested URL is correct, but to verify manually:
1. Enable Add-on Manager logging (extensions.logging.enabled = true)
2. Open Error Console
3. Open Add-ons Manager
4. Search for an addon on AMO, eg "adblock" (bug 706385 isn't fixed yet, so it has to be something that's explicitly compatible with the version of Firefox that's running)
5. Make sure "Available Add-ons" is selected in the search pane that's opened
6. You should see one or more results shown (that verifies that the AMO API is accepting the new parameter, bug 707359)
7. In the Error Console, you should see something like "LOG addons.repository: Requesting https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock/all/30/WINNT/11.0a1/normal?src=firefox"
8. With compatible-by-default enabled, the part of the URL after the application version should be "normal"
9. Repeating with strict compatibility enabled (extensions.strictCompatibility = true), it should be "strict"
10. Repeating with compatibility checking disabled (extensions.checkCompatibility.VERSION = false), it should be "ignore"
Let's leave this [qa?] until the AMO changes are live. Blair, once bug 706385 is resolved please change the whiteboard tag to [qa+] so we can verify it.
Er, scratch that - bug 706385 may be fixed now, but it won't be live yet.
Bug 706385 is now enabled on the development site, so to test this:
* In about:config, change the value of extensions.getAddons.search.url so it's domain is addons-dev.allizom.org (instead of services.addons.mozilla.org)
* Follow steps in comment 7
Given this was being tracked for Firefox 10, we are now 2 releases behind and this is not being tracked by QA any longer. Is this something we can test against Firefox 13 Beta? and if so, should we track it?
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11)
> Is this something we can
> test against Firefox 13 Beta?
Yes, testable on any Firefox since 10. But only now, since it's been waiting on the AMO change.
> and if so, should we track it?
I think so. This bug didn't do anything useful without the AMO change. The API change is already getting QA'ed on AMO's side, but IMO there's some benefit in getting some end-to-end testing done, as opposed to solely testing the discrete parts.
(In reply to Blair McBride (:Unfocused) from comment #12)
> I think so. This bug didn't do anything useful without the AMO change. The
> API change is already getting QA'ed on AMO's side, but IMO there's some
> benefit in getting some end-to-end testing done, as opposed to solely
> testing the discrete parts.
Does your comment 7 test cover the end-to-end scenario you envision? Are there other things we should check to call this verified?
It roughly does. In step 4, search for the name of an addon that wouldn't normally be compatible without compatible-by-default, check that the install succeeds. All addons in the results list should be able to be installed. Just to get some assurance that people are getting the results that are expected (seeing addons that are now compatible, but no addons that aren't compatible).
The AMO site assumes compatible-by-default now, so you can match up what shouldn't by in the search results in the Add-ons Manager by what's listed as not compatible on AMO. though, it's harder for figuring out what is compatible but wasn't before default-to-compatible - if you enable strict compatibility (extensions.strictCompatibility=true) and the addon fails to install, then that's such an addon.
eg: Searching for 'adblock' - AdBlock is compatible with and without compatible-by-default, but AdBlock Lite is only compatible when compatible-by-default is enabled - so shouldn't show in the results when extensions.strictCompatibility=true
And with extensions.checkCompatibility.VERSION=false, the only matching addons that shouldn;'t be in the results are those that aren't compatible with the OS you're on. eg, searching for 'cooliris' on Linux, Cooliris 3d Wall shouldn't show up - but on Windows it should.