-
Notifications
You must be signed in to change notification settings - Fork 998
Send active cohorts to broken report site #6270
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
Send active cohorts to broken report site #6270
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
23a1ab7
to
6bd1ea0
Compare
466f05f
to
a40911e
Compare
8e526a9
8000
to
a9c3065
Compare
@@ -1184,7 +1191,12 @@ class BrowserWebViewClientTest { | |||
var countFinished = 0 | |||
var countStarted = 0 | |||
|
|||
override fun onPageStarted(webView: WebView, url: String?, site: Site?) { |
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.
I'm removing the site here because it was only used to check isDesktopMode
. However, site isn't assigned until after onPageStarted
is called, therefore it was null for new tabs, and contained old data for every subsequent page load. It still worked for isDesktopMode
, because if site is null, we default to setting it as false, and every time we switch between desktop and mobile modes a page refresh is triggered, but can lead to confusion and issues for virtually any other site property
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. 8000
Looks good and works as expected! 🎉
4bce050
to
08d754e
Compare
a9c3065
to
7d65d1e
Compare
7d65d1e
into
feature/cris/css-experiments/send-api-manipulation-json
Task/Issue URL: https://app.asana.com/1/137249556945/project/1205008441501016/task/1209783667491175?focus=true
Description
Send active cohorts for C-S-S experiments to broken site report
Steps to test this PR
Feature 1
epbf
pixel is fired and contains a parametercontentScopeExperiments
with valuebloops:{cohort},test:{cohort}
. Note: cohorts are assigned randomly and therefore the value can't be predictedUI changes