-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stringify webchannel event details on Thunderbird as well as Firefox #17528
Conversation
re-running from failed |
maybe a rebase? I'm not sure why it would be timing out |
9de6d50
to
a583062
Compare
… well as Firefox Because: - the Gecko team want to [remove the webchannel whitelist preference](https://bugzilla.mozilla.org/show_bug.cgi?id=1275612), and - we want to stop using `general.useragent.compatMode.firefox` to add Firefox/x.y to Thunderbird's user-agent string This commit: - changes the user-agent parsing code to detect Thunderbird, and - uses that to fix a block which breaks webchannels on Thunderbird
a583062
to
e2fef1e
Compare
@@ -122,8 +122,9 @@ function formatEventDetail(win, eventDetail) { | |||
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1275616 and | |||
// https://bugzilla.mozilla.org/show_bug.cgi?id=1238128 | |||
if ( | |||
(userAgent.isFirefoxDesktop() || userAgent.isFirefoxAndroid()) && | |||
userAgent.browser.version >= 50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darktrojan the test failures were caused by the removal of the browser version condition. I've restored it to the predicate, as my attempt at updating the related code became more time consuming than I expected, and this is a quicker fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? Everything passed when I rebased it. That's weird. Oh well, as long as it works, and it appears to work.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chenba Thanks!
Originally this was opened to run CI for #17518. But due to time zone differences, FxA fixed the issue that was causing the test failures here. See #17518 for its original description.
closes FXA-10411