@AndyRussG @Pcoombe do you care what we name the JavaScript variable that will indicate to any other RL JS module whether HoverCards is in force?

@dr0ptp4kt For the purposes of fundraising, we're really not interested in tracking users with unique IDs. All we care about is whether they have Hovercards enabled or not when they see a banner. It seems preferable if the Hovercard setting persists across sessions: both from a user experience perspective, and to better model how it would actually interact with fundraising if it was enabled permanently. If that's too difficult, then we can probably manage with a per-session setting.

The technical details of bucketing are definitely more in @AndyRussG's realm.

I don't mind what the variable is named at all.

Thanks, @Pcoombe. Given this, it sounds like the prudent course of action for us is to have a new LocalStorage value for sampling the user in for the feature (assuming she doesn't already have HoverCards enabled via beta features) similar to mobile-betaoptin-token, but don't actually send that value when logging events for EL. If I understand correctly, for CN to do its instrumentation it just needs a JavaScript variable exposed so it can tell whether Hovercards are on or not.

As a side matter, variables like mobile-betaoptin-token are listed at https://wikimediafoundation.org/wiki/Cookie_statement#3._What_types_of_cookies_does_Wikimedia_use.3F and https://wikimediafoundation.org/wiki/Privacy_policy/FAQ#Can_you_give_me_some_examples_of_types_of_cookies_and_how_you_use_local_storage.3F, so we'll need to get those updated presuming we go the route of defining a new LocalStorage variable (e.g., with a key of experiment-token).

@dr0ptp4kt :

Some caveats of local storage:

• It impacts performance as reads are synchronous
• it might not be available if user is browsing in incognito mode or it will get purged when user exits incognito
• items stored there are not encrypted
• storage is shared among different tabs of the same site

See: http://arbitraryreason.com/dont-forget-to-check-private-browsing-mode-when-testing/

Some ideas:
For expirations of data stored there I think sessionstorage is probably a better option than localstorage, when user closes the browser the settings are cleared
If user has cookies off or DNT do not set anything on session/local storage, this would be handled hopefully though some common code in mediawiki
I would store only hovercards=on but that's it. I am not sure a token of any sort is needed.

In T132604#2249669, @Nuria wrote:

Some caveats of local storage:

Also, (just in case this is relevant, and not covered above) "We had to disable localStorage caching in Firefox because of the way it manages quotas." per https://lists.wikimedia.org/pipermail/wikitech-l/2016-April/085380.html (done as part of T66721: mw.loader.store should not occupy all of localStorage IIUC)
Although that's probably covered by

• storage is shared among different tabs of the same site

(Sidenote: This might be relevant or of interest T111144: Gather mw.loader.store (localStorage) statistics from users.)

In T132604#2249669, @Nuria wrote:

@dr0ptp4kt :

Some caveats of local storage:

• It impacts performance as reads are synchronous

Right. From what I've seen there's usually a way to do this where it's not terribly noticeable. That said, fair point. I wonder if a client-side cookie with an expiration date (e.g., 90 days) would be better.

• it might not be available if user is browsing in incognito mode or it will get purged when user exits incognito

Salient point. This probably bolsters the cookie based approach.

• items stored there are not encrypted

Right. It's a random value with the trust boundary terminating at the client for the random value. It doesn't get sent to the server. Of course the device security may or may not have certain controls like disk or per-user encryption and so forth, although that's out of scope.

• storage is shared among different tabs of the same site

That's what we want in this case (we've seen it's expectation defying when the UX changes dramatically over the course of several tabs or browser restarts otherwise). If the user crosses non-private/private boundaries, well, not much we can do there, and that's okay.

See: http://arbitraryreason.com/dont-forget-to-check-private-browsing-mode-when-testing/

Good point! Probably another reason to go cookie.

Some ideas:
For expirations of data stored there I think sessionstorage is probably a better option than localstorage, when user closes the browser the settings are cleared

The eligibility check should cross the session boundary (so that the user has a consistent experience between browser restarts), so I think SessionStorage is ruled out here.

If user has cookies off or DNT do not set anything on session/local storage, this would be handled hopefully though some common code in mediawiki

Transmission of the data to the server endpoint via EL is determined by DNT already. I believe avoiding collection at the server trust boundary is the purpose of DNT? I guess simply keeping users with DNT in the control bucket is one approach. Those users wouldn't have sent EL events anyway. One downside there would be skew on the webrequest logs (lower proportion of Hovercards API requests). Granted, we're looking at EL primarily, it's just that in establishing ratios between webrequest log counts and EL events (e.g., to quickly check that event counts match roughly within expected thresholds) this further amplifies the effect on such ratios due to DNT already.

I would store only hovercards=on but that's it. I am not sure a token of any sort is needed.

The thought was to keep it general. This would have the fringe benefit that if we used a hardcoded pseudorandom seed per experiment it would also be possible to conduct similar work in other contexts.

In T132604#2249737, @Quiddity wrote:

In T132604#2249669, @Nuria wrote:

Some caveats of local storage:

Also, (just in case this is relevant, and not covered above) "We had to disable localStorage caching in Firefox because of the way it manages quotas." per https://lists.wikimedia.org/pipermail/wikitech-l/2016-April/085380.html (done as part of T66721: mw.loader.store should not occupy all of localStorage IIUC)
Although that's probably covered by

• storage is shared among different tabs of the same site

(Sidenote: This might be relevant or of interest T111144: Gather mw.loader.store (localStorage) statistics from users.)

Yeah, it sounds like in this case a cookie is probably a simpler route for this aspect of the work.

Yeah, it sounds like in this case a cookie is probably a simpler route for this aspect of the work.

Actually we want to avoid proliferation of cookies, I think Localstorage is fine, let's just be aware of the points mentioned and make sure the storage is handled with some common code in mw that takes into account FF issues.

Cool, can do.

Hi! Apologies for taking a long time to reply here.

tl;dr: Fully agreed with @Nuria: despite limitations, LocalStorage is the way to go. There are facilities in core and in CentralNotice that might help with this.

In core, we now have mw.requestIdleCallback(), for executing JS code that is not time-sensitive. By using it to wrap calls to LocalStorage, it seems you can reduce the possibility of degrading the UX due to blocking.

Another issue with LocalStorage items is that, unlike cookies, they don't have an expiry time. This has been addressed by a small LocalStorage module that's currently part of CentralNotice. The module, ext.centralNotice.kvStore, adds expiry information to every item stored, and purges expired items shortly after page load (using mw.requestIdleCallback() to avoid blocking).

Something like kvStore may well make it into core pretty soon (see T121646).

In the meantime, the following hack should do the trick (haven't tested it):

mw.requestIdleCallback( function () {
    mw.loader.using( 'ext.centralNotice.kvStore' ).done( function () {
        mw.requestIdleCallback( function () {

             // Check if LocalStorage is available on this client
            var haveStorage = mw.centralNotice..kvStore.isAvailable();

             // Set a value
             // 30 is the duration of the item, in days.
             // After that, it's not retrievable and may be purged from LocalStorage.
             mw.centralNotice.kvStore.setItem( 'key', 'value', mw.centralNotice.kvStore.contexts.GLOBAL, 30 );

             // Retrieve the value
             var value = mw.centralNotice.kvStore.getItem( 'key',  mw.centralNotice.kvStore.contexts.GLOBAL );

             // Check if there were any problems with LocalStorage
             var error = mw.centralNotice.kvStore.getError();
        } );
    } );
} );

The library wraps LocalStorage access in try blocks, so you don't have to worry about exceptions that LocalStorage can throw.

Again, this a hack; parts of the library were not designed to be used outside the context of a CentralNotice campaign. But if you use only the GLOBAL context, as shown, it should work. :)

Regarding incognito/private browsing modes, I don't think there's much, if any, advantages to cookies over LocalStorage.

Hope this is useful! :)

Another issue with LocalStorage items is that, unlike cookies, they don't have an expiry time.

Let me reiterate that I think we should use session storage and have settings expire when user closes browser.

In T132604#2260393, @Nuria wrote:

Another issue with LocalStorage items is that, unlike cookies, they don't have an expiry time.

Let me reiterate that I think we should use session storage and have settings expire when user closes browser.

...just edited my previous comment to show how you can set a non-default expiry time for items in CN's kvStore (LocalStorage-based).

@Pcoombe: How and when is the variable going to be read? Is there going to be an equivalent change to CentralNotice? If so, when in the page lifecycle is CentralNotice going to be using whatever the Popups extension exposes? I ask because I think there's the possibility of a race between each extension's client-side code loading /cc @AndyRussG

@phuedx I'll only be checking it once the user actually clicks the donate button. However I don't know when @AndyRussG will need it for T134286

Change 288893 had a related patch set uploaded (by Phuedx):
Add PopupsExperiment config var

https://gerrit.wikimedia.org/r/288893

Change 288892 had a related patch set uploaded (by Phuedx):
Add ext.popups.experiment module

https://gerrit.wikimedia.org/r/288892

Change 288594 had a related patch set uploaded (by Phuedx):
Add ext.popups.index module

https://gerrit.wikimedia.org/r/288594

Change 288896 had a related patch set uploaded (by Phuedx):
Conditionally enable Popups

https://gerrit.wikimedia.org/r/288896

The BetaFeatures PHPUnit test failure is fixed in 288978: Fix HTMLFeatureFieldTest test cases.

Change 288594 abandoned by Phuedx:
Add ext.popups.index module

Reason:
Per the review of PS6/8.

https://gerrit.wikimedia.org/r/288594

Change 288892 merged by jenkins-bot:
Add ext.popups.experiment module

https://gerrit.wikimedia.org/r/288892

@phuedx do you need review or help with this?

Change 289709 had a related patch set uploaded (by Phuedx):
Handle user explicitly enabling/disabling feature

https://gerrit.wikimedia.org/r/289709

In T132604#2308476, @Jhernandez wrote:

@phuedx do you need review or help with this?

Thanks for the ping @Jhernandez. @Jdlrobson and I are going through this synchronously now.

Change 289709 merged by jenkins-bot:
Handle user explicitly enabling/disabling feature

https://gerrit.wikimedia.org/r/289709

Change 289729 had a related patch set uploaded (by Phuedx):
Convert isUserInCondition from async to sync

https://gerrit.wikimedia.org/r/289729

Change 288896 abandoned by Phuedx:
Conditionally enable Popups

Reason:
This change is superseded by PS13 of Ia71ca92.

https://gerrit.wikimedia.org/r/288896

Change 289729 merged by jenkins-bot:
Convert isUserInCondition from async to sync

https://gerrit.wikimedia.org/r/289729

Change 288893 merged by jenkins-bot:
Conditionally enable Popups

https://gerrit.wikimedia.org/r/288893

How does signoff look like for this task?

Do we need to update the mw.org page with the new config?

Is this config enabled in beta labs?

In T132604#2311914, @Jhernandez wrote:

How does signoff look like for this task?

Do we need to update the mw.org page with the new config?

Yes.

Is this config enabled in beta labs?

No. Moreover, the experiment is disabled by default. Once I can SSH into any wmflabs bastion host I'll update the configuration on the staging server.

I've enabled the experiment on [our staging server](http://reading-web-staging.wmflabs.org].

• Users who have HoverCards enabled by the beta features should continue to have them enabled

Couldn't test this out, will try removing the experiment, enabling beta feature and then activating the experiment.

• the ability to do a partial roll-out, with sticky bucketing at the user (using device, browser as a proxy). In other words, each user is either in or out when using the same browser on the same device. This is required for the fundraising test we want to do before broader hovercard rollout.

Seems to hold up in my tests.

Note that logged-in users are also affected by this. If you enable hovercards via footer link or experiment, it is just for that browser. A new browser won't remember your setting even if you log in.

• Exposure of a JavaScript variable for other JavaScript to determine whether Hovercards is enabled for the given user. This is the tangible thing that will make it possible for central notice to detect if the user is in the hovercards bucket or control (regardless of whether user has subsequently enabled hovercards)

mw.popups.enabled is exposed and false when not bucketed, and true when bucketed.

@phuedx I've added $wgPopupsBetaFeature = true; to the config on staging to enable the beta feature for the QAing. Will we disable this one so that user's can't enable in preferences or will we keep it in the test?

In T132604#2322207, @Jhernandez wrote:

• Users who have HoverCards enabled by the beta features should continue to have them enabled

Couldn't test this out, will try removing the experiment, enabling beta feature and then activating the experiment.

QAd, if not bucketed, but you log in and you had the beta feature enabled, then popups are enabled.

Also QAd that if you disable the beta feature and you're not bucketed then don't see the popups.

In T132604#2322253, @Jhernandez wrote:

@phuedx I've added $wgPopupsBetaFeature = true; to the config on staging to enable the beta feature for the QAing. Will we disable this one so that user's can't enable in preferences or will we keep it in the test?

I thought we wanted to keep that flag enabled while the test is running. @dr0ptp4kt: Can you confirm whether the beta feature should be disabled while the A/B test is enabled?

From the POV of the fundraising test, it doesn't matter if logged-in users opt into the beta feature. We'll only be showing banners to anonymous users.

@phuedx, logged in users should continue to be able to enable or disable the Hovercards beta feature (and we should honor whatever setting they have). In the case of hu.wikipedia.org Hovercards is one of the available beta features (hu.wikipedia.org says 332 users have it enabled). All clear?

In T132604#2323644, @dr0ptp4kt wrote:

@phuedx, logged in users should continue to be able to enable or disable the Hovercards beta feature (and we should honor whatever setting they have). In the case of hu.wikipedia.org Hovercards is one of the available beta features (hu.wikipedia.org says 332 users have it enabled). All clear?

All clear /cc @Jhernandez

@dr0ptp4kt Something to keep in mind when we rollout (keep the $wgPopupsBetaFeature = true; when adding the experiment config). Clear