Remove MSE debug User Agent string

VERIFIED FIXED in Firefox 37

Status

()

P2
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

(Blocks: 1 bug)

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37+ fixed, firefox38+ verified, firefox39 fixed, firefox40 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Bug 1127448 added a build id to the user agent string when visiting youtube with MSE enabled to facilitate comparing failure logs. This must be removed once MSE is stable and shipping.
Blocks: 778617
Hardware: x86 → All
Posted patch Disable everywhere. (obsolete) — Splinter Review
Bug 1127448 makes this fail on desktop too. I propose we just disable the test there. That change should be temporary while 37 and maybe 38 are in beta, and we want it for testing a high-priority feature.
Attachment #8565736 - Flags: review?(ryanvm)
Comment on attachment 8565736 [details] [diff] [review]
Disable everywhere.

Not sure who the right person to rubberstamp this is, but I'm pretty sure it ain't me.
Attachment #8565736 - Flags: review?(ryanvm)
Comment on attachment 8565736 [details] [diff] [review]
Disable everywhere.

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #2)

> Not sure who the right person to rubberstamp this is, but I'm pretty sure it
> ain't me.

:P

Trying module owner. Patrick, inbound didn't explode here, so maybe this is optional, but I fear we'll need it on 37.
Attachment #8565736 - Flags: review?(mcmanus)
content of UA strings isn't me - its usually :gerv fwiw

but I'm happy to rs the testing change for the moment.
Comment on attachment 8565736 [details] [diff] [review]
Disable everywhere.

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

rs=mcmanus
Attachment #8565736 - Flags: review?(mcmanus) → review+
Thanks, this bug is just about the test issue.

Ryan, I suggest we land this on 37 only unless central starts failing.
Sorry, this patch should have been attached to bug 1111137.

Updated

4 years ago
Priority: -- → P2
Ralph - This override needs to be removed in Beta 7 (gtb tomorrow). Can you please request uplift and coordinate this change with YouTube?

ni Sheila to add this to the MSE list if it's not already on it.
status-firefox37: --- → affected
status-firefox38: --- → affected
tracking-firefox37: --- → +
tracking-firefox38: --- → +
Flags: needinfo?(smooney)
Flags: needinfo?(giles)
Ralph is away. Transferring ni to cpearce.
Flags: needinfo?(giles) → needinfo?(cpearce)
Assignee: nobody → cpearce
I need a fix for this bug for Beta 7 today. I expected to have one this morning.

ni Anthony, Jet, Chris for help getting this cleaned up today.
Flags: needinfo?(cpeterson)
Flags: needinfo?(bugs)
Flags: needinfo?(ajones)
The YouTube user agent override is #ifdef'd only for MOZ_UPDATE_CHANNEL != release, so the user agent override should not 'leak' into the release channel. Still, we will should back it out so the final Beta 37 build has the same configuration as the release channel.

https://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#126
Flags: needinfo?(cpeterson)
I agree with cpeterson. The MSE debug info in the UA string is should not leak into release.

I can still prepare a backout for all branches if that's what people want.
Discussed with lmandel on IRC. We'll wontfix this for 37, since the code should not be compiled/defined in a release channel build.

We should backout Bug 1133862 from aurora and central, assuming it's of no value there.
status-firefox37: affected → wontfix
status-firefox39: --- → affected
Flags: needinfo?(ajones)
Let's call this fixed instead so that we can verify...
status-firefox37: wontfix → fixed
Flags: qe-verify+
Flags: needinfo?(bugs)
Stealing. We should start turning this off now that we've rolled out to release.
Assignee: cpearce → giles
Flags: needinfo?(smooney)
Flags: needinfo?(cpearce)
Attachment #8565736 - Attachment is obsolete: true
Attachment #8587066 - Flags: review?(mconley)
Comment on attachment 8587066 [details] [diff] [review]
Remove MSE debug user agent

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

Thanks!
Attachment #8587066 - Flags: review?(mconley) → review+
Oops, not fixed until it lands on m-c.
status-firefox40: fixed → affected
https://hg.mozilla.org/mozilla-central/rev/4d6d69f0f499
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Does anything need manual testing here?
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #21)
> Does anything need manual testing here?

It's worth verifying. Visit youtube and confirm we send the same User Agent to youtube and non-youtube domains, e.g. by looking at the request headers in the network tab of the developer tools.
Keywords: verifyme
Comment on attachment 8587066 [details] [diff] [review]
Remove MSE debug user agent

Approval Request Comment
[Feature/regressing bug #]: Bug 1127448/MSE
[User impact if declined]: Build ids shared with google when visiting youtube with non-release builds, reducing privacy.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Risk is low. This patch just reverts the testing code and restores normal behaviour.
[String/UUID change made/needed]: None.
Attachment #8587066 - Flags: approval-mozilla-beta?
Attachment #8587066 - Flags: approval-mozilla-aurora?
Attachment #8587066 - Flags: approval-mozilla-beta?
Attachment #8587066 - Flags: approval-mozilla-beta+
Attachment #8587066 - Flags: approval-mozilla-aurora?
Attachment #8587066 - Flags: approval-mozilla-aurora+
Verified as fixed with Firefox 38 beta 6 (BuildID: 20150420134330) and Nightly 40.0a1 (2015-04-22), under OSX 10.9.5, Windows 7 64 bit and Linux 14.04 32 bit - the same User Agent is sent to youtube and non-youtube domains, the Build ID is no longer displayed.
Status: RESOLVED → VERIFIED
status-firefox38: fixed → verified
status-firefox40: fixed → verified
I think verification on 40 Nightly and 38 Beta should suffice here.
Flags: qe-verify+
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.