Build A/B test bucketing infrastructure for the table of contents.

* Bucket and sample on server by using the
  `WikimediaEvents.WebABTestArticleIdFactory` service from
  WikimediaEvents (soft dependency)
* Add linkHijack.js so that users bucketed in one group have the
  possibility of remaining in that group if they click a link to another
  page.

Bug: T302046
Depends-On: Ie6627de98effb3d37a3bedda5023d08af319837f
Change-Id: Iff231a976c473217b0fa4da1aa9a8d1c2a1a19f2
This commit is contained in:
Nicholas Ray 2022-03-17 17:02:39 -06:00
parent 19f114281a
commit 6fbf08a198
9 changed files with 381 additions and 26 deletions

View File

@ -173,12 +173,11 @@ final class Constants {
public const FEATURE_STICKY_HEADER_EDIT = 'StickyHeaderEdit';
/**
* Defines whether the Sticky Header A/B test is running. See
* https://phabricator.wikimedia.org/T292587 for additional detail about the test.
* Defines whether an A/B test is running.
*
* @var string
*/
public const CONFIG_STICKY_HEADER_TREATMENT_AB_TEST_ENROLLMENT = 'VectorWebABTestEnrollment';
public const CONFIG_WEB_AB_TEST_ENROLLMENT = 'VectorWebABTestEnrollment';
/**
* The `mediawiki.searchSuggest` protocol piece of the SearchSatisfaction instrumention reads
@ -270,6 +269,11 @@ final class Constants {
*/
public const FEATURE_TABLE_OF_CONTENTS = 'TableOfContents';
/**
* @var string
*/
public const WEB_AB_TEST_ARTICLE_ID_FACTORY_SERVICE = 'WikimediaEvents.WebABTestArticleIdFactory';
/**
* This class is for namespacing constants only. Forbid construction.
* @throws FatalError

View File

@ -40,7 +40,7 @@ class Hooks {
*/
private static function getActiveABTest( $config ) {
$ab = $config->get(
Constants::CONFIG_STICKY_HEADER_TREATMENT_AB_TEST_ENROLLMENT
Constants::CONFIG_WEB_AB_TEST_ENROLLMENT
);
if ( count( $ab ) === 0 ) {
// If array is empty then no experiment and need to validate.
@ -460,6 +460,59 @@ class Hooks {
}
}
/**
* Returns the necessary TOC classes.
*
* @param Skin $sk
* @param Config $config
* @return string[]
*/
private static function getTocClasses( Skin $sk, $config ): array {
if ( !( $sk instanceof SkinVector22 ) ) {
return [];
}
$classes = [];
$isTocABTestEnabled = $sk->isTOCABTestEnabled();
if ( $isTocABTestEnabled || $sk->isTOCEnabled() ) {
$classes[] = 'vector-toc-enabled';
}
if (
$sk->isTableOfContentsVisibleInSidebar() &&
$isTocABTestEnabled
) {
/** @var \WikimediaEvents\WebABTest\WebABTestArticleIdFactory */
$webABTestArticleIdFactory = MediaWikiServices::getInstance()->getService(
Constants::WEB_AB_TEST_ARTICLE_ID_FACTORY_SERVICE
);
$experimentConfig = $config->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
$bucketKeys = array_keys( $experimentConfig['buckets'] );
$ab = $webABTestArticleIdFactory->makeWebABTestArticleIdStrategy(
$webABTestArticleIdFactory->filterExcludedBucket( $bucketKeys ),
1 - $experimentConfig['buckets']['unsampled']['samplingRate'],
Constants::QUERY_PARAM_TABLE_OF_CONTENTS,
$sk->getContext()
);
if ( !$ab ) {
return $classes;
}
$bucket = $ab->getBucket();
if ( $bucket ) {
$experimentName = $experimentConfig[ 'name' ];
$classes[] = $experimentName;
$classes[] = "$experimentName-$bucket";
}
}
return $classes;
}
/**
* Called when OutputPage::headElement is creating the body tag to allow skins
* and extensions to add attributes they might need to the body of the page.
@ -473,6 +526,7 @@ class Hooks {
if ( !self::isVectorSkin( $skinName ) ) {
return;
}
$config = $sk->getConfig();
// As of 2020/08/13, this CSS class is referred to by the following deployed extensions:
//
@ -486,7 +540,11 @@ class Hooks {
$bodyAttrs['class'] .= ' skin-vector-legacy';
}
$config = $sk->getConfig();
$tocClasses = self::getTocClasses( $sk, $config );
if ( $tocClasses ) {
$bodyAttrs['class'] .= ' ' . implode( ' ', $tocClasses );
}
// Should we disable the max-width styling?
if ( !self::isSkinVersionLegacy( $skinName ) && $sk->getTitle() && self::shouldDisableMaxWidth(
$config->get( 'VectorMaxWidthOptions' ),

View File

@ -118,7 +118,6 @@ abstract class SkinVector extends SkinMustache {
private const SEARCH_SHOW_THUMBNAIL_CLASS = 'vector-search-box-show-thumbnail';
private const SEARCH_AUTO_EXPAND_WIDTH_CLASS = 'vector-search-box-auto-expand-width';
private const STICKY_HEADER_ENABLED_CLASS = 'vector-sticky-header-enabled';
private const TABLE_OF_CONTENTS_ENABLED_CLASS = 'vector-toc-enabled';
private const CLASS_QUIET_BUTTON = 'mw-ui-button mw-ui-quiet';
private const CLASS_PROGRESSIVE = 'mw-ui-progressive';
private const CLASS_ICON_BUTTON = 'mw-ui-icon mw-ui-icon-element';
@ -405,10 +404,6 @@ abstract class SkinVector extends SkinMustache {
$original['class'] = implode( ' ', [ $original['class'] ?? '', self::STICKY_HEADER_ENABLED_CLASS ] );
}
if ( VectorServices::getFeatureManager()->isFeatureEnabled( Constants::FEATURE_TABLE_OF_CONTENTS ) ) {
$original['class'] = implode( ' ', [ $original['class'] ?? '', self::TABLE_OF_CONTENTS_ENABLED_CLASS ] );
}
return $original;
}

View File

@ -2,12 +2,16 @@
namespace Vector;
use MediaWiki\MediaWikiServices;
/**
* @ingroup Skins
* @package Vector
* @internal
*/
class SkinVector22 extends SkinVector {
private const TOC_AB_TEST_NAME = 'skin-vector-toc-experiment';
/**
* Updates the constructor to conditionally disable table of contents in article
* body. Note, the constructor can only check feature flags that do not vary on
@ -15,22 +19,62 @@ class SkinVector22 extends SkinVector {
* @inheritDoc
*/
public function __construct( array $options ) {
$options['toc'] = !$this->isTableOfContentsVisibleInSidebar();
if ( !$this->isTOCABTestEnabled() ) {
$options['toc'] = !$this->isTableOfContentsVisibleInSidebar();
}
parent::__construct( $options );
}
/**
* @internal
* @return bool
*/
public function isTOCABTestEnabled(): bool {
$experimentConfig = $this->getConfig()->get( Constants::CONFIG_WEB_AB_TEST_ENROLLMENT );
return $experimentConfig['name'] === self::TOC_AB_TEST_NAME &&
$experimentConfig['enabled'] &&
MediaWikiServices::getInstance()->hasService( Constants::WEB_AB_TEST_ARTICLE_ID_FACTORY_SERVICE );
}
/**
* Returns whether or not the table of contents is enabled through
* FeatureManager.
*
* @internal
* @return bool
*/
public function isTOCEnabled() {
$featureManager = VectorServices::getFeatureManager();
return $featureManager->isFeatureEnabled( Constants::FEATURE_TABLE_OF_CONTENTS );
}
/**
* Determines if the Table of Contents should be visible.
* TOC is visible on main namespaces except for the Main Page
* when the feature flag is on.
*
* @internal
* @return bool
*/
private function isTableOfContentsVisibleInSidebar(): bool {
$featureManager = VectorServices::getFeatureManager();
public function isTableOfContentsVisibleInSidebar(): bool {
$title = $this->getTitle();
$isMainPage = $title ? $title->isMainPage() : false;
return $featureManager->isFeatureEnabled( Constants::FEATURE_TABLE_OF_CONTENTS ) && !$isMainPage;
if (
!$title ||
$title->getArticleID() === 0 ||
$title->isMainPage()
) {
return false;
}
if ( $this->isTOCABTestEnabled() ) {
return true;
}
return $this->isTOCEnabled();
}
/**

View File

@ -0,0 +1,71 @@
/**
* Appends a query param to the `href` attribute of any clicked anchor element
* or anchor element that is the parent of a clicked HTMLElement. Links that
* lead to different origins or have the same pathname and have a hash fragment
* are ignored.
*
* @param {string} key
* @param {string} value
* @return {Function} A cleanup function is returned that
* removes any event listeners that were added.
*/
function linkHijack( key, value ) {
/**
* @param {MouseEvent} e
*/
function handleClick( e ) {
if ( !e.target || !( e.target instanceof HTMLElement ) ) {
return;
}
const anchor = e.target.closest( 'a' );
if ( !anchor ) {
return;
}
let locationUrl;
let oldUrl;
try {
// eslint-disable-next-line compat/compat
locationUrl = new URL( location.href );
// eslint-disable-next-line compat/compat
oldUrl = new URL( anchor.href );
} catch ( error ) {
// A TypeError may be thrown for invalid URLs. In that case, return
// gracefully.
return;
}
if (
locationUrl.origin !== oldUrl.origin ||
( locationUrl.pathname === oldUrl.pathname && oldUrl.hash )
) {
// Return early if link leads to host outside the current one or if the
// url contains a pathname that is the same as the current one and also
// has a hash fragment (e.g. this occurs with links in the TOC and we
// don't want to trigger a refresh of the page by appending a query
// param).
return;
}
// eslint-disable-next-line compat/compat
const params = new URLSearchParams( oldUrl.search );
if ( !params.has( key ) ) {
params.append( key, value );
}
// eslint-disable-next-line compat/compat
const newUrl = new URL( `${oldUrl.origin}${oldUrl.pathname}?${params}${oldUrl.hash}` );
anchor.setAttribute( 'href', newUrl.toString() );
}
document.body.addEventListener( 'click', handleClick );
return () => {
document.body.removeEventListener( 'click', handleClick );
};
}
module.exports = linkHijack;

View File

@ -7,6 +7,8 @@ const
initSectionObserver = require( './sectionObserver.js' ),
initTableOfContents = require( './tableOfContents.js' ),
deferUntilFrame = require( './deferUntilFrame.js' ),
linkHijack = require( './linkHijack.js' ),
ABTestConfig = require( /** @type {string} */ ( './config.json' ) ).wgVectorWebABTestEnrollment || {},
TOC_ID = 'mw-panel-toc',
TOC_ID_LEGACY = 'toc',
BODY_CONTENT_ID = 'bodyContent',
@ -15,7 +17,8 @@ const
TOC_LEGACY_PLACEHOLDER_TAG = 'mw:tocplace',
TOC_SCROLL_HOOK = 'table_of_contents',
PAGE_TITLE_SCROLL_HOOK = 'page_title',
ABTestConfig = require( /** @type {string} */ ( './config.json' ) ).wgVectorWebABTestEnrollment || {};
TOC_QUERY_PARAM = 'tableofcontents',
TOC_EXPERIMENT_NAME = 'skin-vector-toc-experiment';
/**
* @callback OnIntersection
@ -144,8 +147,26 @@ const main = () => {
tocElement &&
bodyContent &&
window.IntersectionObserver &&
window.requestAnimationFrame )
) {
window.requestAnimationFrame
) ) {
return;
}
const experiment =
!!ABTestConfig.enabled &&
ABTestConfig.name === TOC_EXPERIMENT_NAME &&
document.body.classList.contains( ABTestConfig.name ) &&
// eslint-disable-next-line compat/compat
window.URLSearchParams &&
initExperiment( ABTestConfig );
const isInTreatmentBucket = !!experiment && experiment.isInTreatmentBucket();
if ( experiment && experiment.isInSample() ) {
linkHijack( TOC_QUERY_PARAM, isInTreatmentBucket ? '1' : '0' );
}
if ( experiment && !isInTreatmentBucket ) {
// Return early if the old TOC is shown.
return;
}

View File

@ -267,6 +267,12 @@ body {
.transform( translateX( -( @max-width-page-container - @max-width-workspace-container ) / 2 ) );
}
.skin-vector-toc-experiment-control .mw-table-of-contents-container,
.skin-vector-toc-experiment-unsampled .mw-table-of-contents-container,
.skin-vector-toc-experiment-treatment #toc {
display: none;
}
// Hide sidebar entirely when the checkbox is disabled and the TOC is enabled
.vector-toc-enabled #mw-sidebar-checkbox:not( :checked ) ~ .mw-workspace-container .mw-sidebar {
display: none;

View File

@ -285,6 +285,7 @@
"resources/skins.vector.es6/tableOfContents.js",
"resources/skins.vector.es6/sectionObserver.js",
"resources/skins.vector.es6/deferUntilFrame.js",
"resources/skins.vector.es6/linkHijack.js",
{
"name": "resources/skins.vector.es6/config.json",
"callback": "Vector\\Hooks::getVectorResourceLoaderConfig"
@ -467,20 +468,17 @@
},
"VectorWebABTestEnrollment": {
"value": {
"name": "vector.sticky_header",
"name": "skin-vector-toc-experiment",
"enabled": false,
"buckets": {
"unsampled": {
"samplingRate": 0.1
"samplingRate": 0.8
},
"control": {
"samplingRate": 0.3
"samplingRate": 0.1
},
"stickyHeaderDisabled": {
"samplingRate": 0.3
},
"stickyHeaderEnabledTreatment": {
"samplingRate": 0.3
"treatment": {
"samplingRate": 0.1
}
}
},

View File

@ -0,0 +1,158 @@
const linkHijack = require( '../../resources/skins.vector.es6/linkHijack.js' );
describe( 'linkHijack.js', () => {
let /** @type {jest.Mock} */ getHrefSpy;
let /** @type {Function | undefined} */ cleanup;
beforeEach( () => {
cleanup = undefined;
getHrefSpy = jest.fn( () => 'http://localhost/wiki/Tree' );
// @ts-ignore
delete window.location;
// @ts-ignore
window.location = {
get href() {
return getHrefSpy();
}
};
document.body.innerHTML = `
<a class="anchor" href="/wiki/Barack_Obama">Barack Obama</a>
<a class="anchor-with-span" href="/wiki/Barack_Obama"><span class="inner-span">Barack Obama</span></a>
<a class="anchor-with-hash" href="/wiki/Tree#jump">Jump</a>
<a class="anchor-with-different-origin" href="http://www.google.com/">Different Origin</a>
<a class="anchor-with-query-param" href="/wiki/Barack_Obama?tableofcontents=1">Query Param</a>
<a class="anchor-with-different-query-param" href="/wiki/Barack_Obama?foo=1">Query Param</a>
<a class="anchor-without-href">Anchor without href</a>
<a class="anchor-with-invalid-href href="invalid">Anchor with invalid href</a>
<div></div>
<svg></svg>
`;
} );
afterEach( () => {
if ( cleanup ) {
cleanup();
}
} );
describe( 'when link origin is same and pathname is different', () => {
it( 'appends a query param to anchor element when user clicks anchor element', () => {
const anchor = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchor.click();
expect( anchor.href ).toBe( 'http://localhost/wiki/Barack_Obama?tableofcontents=1' );
} );
it( 'appends a query param to anchor element when user clicks inner span element', () => {
const anchorWithSpan = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor-with-span' ) );
const innerSpan = /** @type {HTMLSpanElement} */ ( document.querySelector( '.inner-span' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
innerSpan.click();
expect( anchorWithSpan.href ).toBe( 'http://localhost/wiki/Barack_Obama?tableofcontents=1' );
} );
} );
describe( 'when link origin is different and pathname is different', () => {
it( 'does not append a query param to the anchor element', () => {
const anchorWithDifferentOrigin = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor-with-different-origin' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchorWithDifferentOrigin.click();
expect( anchorWithDifferentOrigin.href ).toBe( 'http://www.google.com/' );
} );
} );
describe( 'when link origin is same and pathname is same and hash fragment is present', () => {
it( 'does not append a query param to the anchor element', () => {
const anchorWithHash = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor-with-hash' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchorWithHash.click();
expect( anchorWithHash.href ).toBe( 'http://localhost/wiki/Tree#jump' );
} );
} );
describe( 'when link already has the same query param', () => {
it( 'does not duplicate query params', () => {
const anchorWithQueryParam = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor-with-query-param' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchorWithQueryParam.click();
expect( anchorWithQueryParam.href ).toBe( 'http://localhost/wiki/Barack_Obama?tableofcontents=1' );
} );
} );
describe( 'when link already has different query param', () => {
it( 'appends query param', () => {
const anchorWithDifferentQueryParam = /** @type {HTMLAnchorElement} */ ( document.querySelector( '.anchor-with-different-query-param' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchorWithDifferentQueryParam.click();
expect( anchorWithDifferentQueryParam.href ).toBe( 'http://localhost/wiki/Barack_Obama?foo=1&tableofcontents=1' );
} );
} );
describe( 'when clicking on an element that is not an anchor element or a child of an anchor', () => {
it( 'does nothing (no errors)', () => {
const div = /** @type {HTMLDivElement}} */ ( document.querySelector( 'div' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
div.click();
} );
} );
describe( 'when clicking on an element that is not an HTMLElement', () => {
it( 'does nothing (no errors)', () => {
const svg = /** @type {SVGElement}} */ ( document.querySelector( 'svg' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
svg.dispatchEvent( new Event( 'click', { bubbles: true } ) );
} );
} );
describe( 'when clicking on an Anchor without an `href', () => {
it( 'handles it gracefully (no errors)', () => {
const anchor = /** @type {HTMLAnchorElement}} */ ( document.querySelector( '.anchor-without-href' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchor.click();
} );
} );
describe( 'when clicking on an Anchor with an invalid `href', () => {
it( 'handles it gracefully (no errors)', () => {
const anchor = /** @type {HTMLAnchorElement}} */ ( document.querySelector( '.anchor-with-invalid-href' ) );
cleanup = linkHijack( 'tableofcontents', '1' );
anchor.click();
} );
} );
describe( 'when cleanup function is called', () => {
let /** @type {any} */ events;
beforeEach( () => {
events = {};
jest.spyOn( document.body, 'addEventListener' ).mockImplementation( ( event ) => {
if ( !( event in events ) ) {
events[ event ] = 1;
} else {
events[ event ] += 1;
}
} );
jest.spyOn( document.body, 'removeEventListener' ).mockImplementation( ( event ) => {
events[ event ] -= 1;
if ( events[ event ] === 0 ) {
delete events[ event ];
}
} );
} );
afterEach( () => {
jest.restoreAllMocks();
} );
it( 'removes added event listeners', () => {
linkHijack( 'tableofcontents', '1' )();
linkHijack( 'tableofcontents', '1' )();
expect( Object.keys( events ).length ).toBe( 0 );
} );
} );
} );