Bedrock: Custom stub-attribution code on /new overwrites AMO data

[alexgibson]

Description

The AMO team rely on stub attribution for their "Return to AMO" functionality. The custom stub-attribution script that is currently being used for a first-party experiment on /new can override this data, so we need to add an extra check for utm_source=addons.mozilla.org to avoid clobbering the data.

Steps to reproduce (via mixedpuppy on Slack)

1. install chrome, ensure no browser data/cookies and DNT is off, open network monitor from chrome devtools
2. go to AMO, then ublock origin
3. click the get firefox button
4. land on https://www.mozilla.org/en-US/firefox/new/?utm_campaign=non-fx-button&utm_content=rta%3AdUJsb2NrMEByYXltb25kaGlsbC5uZXQ&utm_medium=referral&utm_source=addons.mozilla.org
5. click on download button
6. download starts, landing me on https://www.mozilla.org/en-US/firefox/download/thanks/
7. network monitor shows request for https://download.mozilla.org/?product=firefox-stub&os=win&lang=en-US
   * it should look like https://download.mozilla.org/?product=firefox-stubl&os=win&lang=en-US&attribution_code={the data}&attribution_sig={the signature})
8. install firefox
9. check for ~/AppData/Local/Mozilla/Firefox/postSigningData, it should exist

Expected result

postSigningData should exist

Actual result

It does not exist

Environment

Windows

[hoosteeno]

@alexgibson I thought the specs we landed on for first-party stub attribution would preserve any and all existing UTM parameters?

• If there is no utm_content param, then we can add it, with a value that includes the experiment identifier and the variation identifier as in utm_content=download_thanks_install_experiment-v-{a|b|c|d}
• If we add a utm_content param, we must ensure that utm_source, utm_medium and utm_content are also populated. If any are missing then we can fill them with default values, e.g. utm_source=www.mozilla.org, utm_medium=download_button{or other ui treatment},
  utm_campaign=download_thanks_page{or other campaign}.
• We should not overwrite utm parameters that exist.

Another expression of the logic is in the pseudocode from our original work on this.

We should not clobber any existing utm parameters, regardless of source. Can we generalize the solution so that any existing source is preserved; any existing campaign is preserved; any existing medium is preserved; and any existing content is preserved? If content is present, we can't use the session in a first party experiment; if content is missing, we can add our experiment's content; and then we need to also be sure the other 3 exist with their previous values or (if no previous values) with our default values.

Should I file a new issue with this information?

[alexgibson]

@hoosteeno thanks this was missed in the original implementation. We didn’t clobber params, but then they weren’t being passed through correctly. It’s now been fixed, together with an added exception for AMO.