Closed Bug 1103216 Opened 5 years ago Closed 5 years ago

Remove affiliate codes from default Google plugin in en-US

Categories

(Firefox :: Search, defect, P1, critical)

34 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 37
Tracking Status
firefox34 + fixed
firefox35 + fixed
firefox36 + fixed
firefox37 --- fixed
firefox-esr31 35+ fixed

People

(Reporter: kev, Assigned: kev)

References

Details

Attachments

(3 files)

Attached patch google-us.patchSplinter Review
Per expiration of the search agreement, the Google affiliate codes must be removed from any and all official builds. Note that Google plugins also exist in the JP and KU locales, and also need to be modified.
Attachment #8527051 - Flags: review?(gavin.sharp)
(In reply to Kev Needham [:kev] from comment #0)
> Note that Google plugins also
> exist in the JP and KU locales, and also need to be modified.

Flod, can we do that for 34? (Separate bug perhaps)
Flags: needinfo?(francesco.lodolo)
We will also need to adjust browser/components/search/test/browser_google_behavior.js and browser/components/search/test/browser_google.js accordingly.
Ah, bug 1103229 is already on file for ku.
For Japan, preferred approach would be to remove all the google plugins and reference the default, which Google has requested multiple times over the year.
Comment on attachment 8527051 [details] [diff] [review]
google-us.patch

f?mfinkle for the mobile/ change.

It feels like there might be some value in leaving in a "from Firefox" generic parameter here, but I guess I don't feel strongly.

We will also need the test changes from comment 2.

I assume you are taking care of looping in whoever needs to be looped in to ensure it is consistent with partner expectations and communicate it to them as needed.
Attachment #8527051 - Flags: review?(gavin.sharp)
Attachment #8527051 - Flags: review+
Attachment #8527051 - Flags: feedback?(mark.finkle)
mconnor and I talked briefly about it, but cc'ing him and Joanne to make sure they close the loop with Google. mconnor - could you verify here when we've got an Ack from Google that the change meets obligations, please and thanks?
Flags: needinfo?(mconnor)
wrt value around leaving client="firefox" in, I'll defer to BD there. They can get the same info from UA analysis, and we don't see any benefit from it that I'm aware of, but users may if it triggers anything on page rendering. Good item to also follow up with them on.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #1)
> Flod, can we do that for 34? (Separate bug perhaps)

I don't think so, Firefox 35 is more likely. I think we're going to have a RC on Monday, but pushing this change doesn't feel safe (CCing also Pike).

* Japanese is up to date with sign-offs, so I don't see any issue with shipping in Fx35, even happier if we drop the Japanese's variant all together.
* Kurdish is quite behind with sign-offs, not sure if we're in shape to take a sign-off on beta next cycle with the updated searchplugin. Let's continue the discussion about Kurdish in bug 1103229.
Flags: needinfo?(francesco.lodolo)
Francesco, if there's any way we can fix ja we should do so, even if we need extra QA from MozJP people. This is a requirement.

For everything else, this is 100% good to go.  No notification needed.
Flags: needinfo?(mconnor)
Blocks: 1103970
Attachment #8527051 - Flags: feedback?(mark.finkle) → feedback+
Attached patch test fixesSplinter Review
Attachment #8527742 - Flags: review?(felipc)
Attachment #8527742 - Flags: review?(dolske)
Attachment #8527742 - Flags: review?(felipc) → review+
Assignee: nobody → kev
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b232181e8006

https://hg.mozilla.org/mozilla-central/rev/99a61ec93c5a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 34 → Firefox 37
We need to remove the Google affiliate codes from Firefox ESR builds as well. Does this patch apply cleanly to the ESR31 branch?

Gavin/Ryan - Can you handle landing this today/tomorrow?

Ben - We should hold off on the ESR build until we can remove the codes.
Flags: needinfo?(ryanvm)
Flags: needinfo?(gavin.sharp)
Flags: needinfo?(bkerensa)
Needs rebasing for esr31 AFAICT.
Flags: needinfo?(ryanvm)
Attached patch esr31 patchSplinter Review
Flags: needinfo?(gavin.sharp)
Attachment #8544245 - Flags: approval-mozilla-esr31?
Attachment #8544245 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Test failures :(
https://treeherder.mozilla.org/logviewer.html#?job_id=42843&repo=mozilla-esr31
Flags: needinfo?(bkerensa) → needinfo?(gavin.sharp)
You need to log in before you can comment on or make changes to this bug.