Skip to content

Commit c33eb3b

Browse files
committed
Bug 1876880 - put the randomization of Report Broken Site reason-dropdown behind a pref, and base its ordering on ClientEnvironment.randomizationId; r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D199790
1 parent 22250c5 commit c33eb3b

File tree

3 files changed

+156
-42
lines changed

3 files changed

+156
-42
lines changed

browser/app/profile/firefox.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2973,6 +2973,8 @@ pref("ui.new-webcompat-reporter.send-more-info-link", false);
29732973
# 0 = disabled, 1 = reason optional, 2 = reason required.
29742974
pref("ui.new-webcompat-reporter.reason-dropdown", 0);
29752975

2976+
pref("ui.new-webcompat-reporter.reason-dropdown.randomized", true);
2977+
29762978
// Reset Private Browsing Session feature
29772979
#if defined(NIGHTLY_BUILD)
29782980
pref("browser.privatebrowsing.resetPBM.enabled", true);

browser/components/reportbrokensite/ReportBrokenSite.sys.mjs

Lines changed: 77 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,20 @@ const DEFAULT_NEW_REPORT_ENDPOINT = "https://webcompat.com/issues/new";
88

99
import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
1010

11+
const lazy = {};
12+
13+
ChromeUtils.defineESModuleGetters(lazy, {
14+
ClientEnvironment: "resource://normandy/lib/ClientEnvironment.sys.mjs",
15+
});
16+
1117
const gDescriptionCheckRE = /\S/;
1218

1319
class ViewState {
1420
#doc;
1521
#mainView;
1622
#reportSentView;
23+
#reasonOptions;
24+
#randomizeReasons = false;
1725

1826
currentTabURI;
1927
currentTabWebcompatDetailsPromise;
@@ -29,6 +37,11 @@ class ViewState {
2937
"report-broken-site-popup-reportSentView"
3038
);
3139
ViewState.#cache.set(doc, this);
40+
41+
this.#reasonOptions = Array.from(
42+
// Skip the first option ("choose reason"), since it always stays at the top
43+
this.reasonInput.querySelectorAll(`menuitem:not(:first-of-type)`)
44+
);
3245
}
3346

3447
static #cache = new WeakMap();
@@ -102,6 +115,46 @@ class ViewState {
102115
);
103116
}
104117

118+
#randomizeReasonsOrdering() {
119+
// As with QuickActionsLoaderDefault, we use the Normandy
120+
// randomizationId as our PRNG seed to ensure that the same
121+
// user should always get the same sequence.
122+
const seed = [...lazy.ClientEnvironment.randomizationId]
123+
.map(x => x.charCodeAt(0))
124+
.reduce((sum, a) => sum + a, 0);
125+
126+
const items = [...this.#reasonOptions];
127+
this.#shuffleArray(items, seed);
128+
items[0].parentNode.append(...items);
129+
}
130+
131+
#shuffleArray(array, seed) {
132+
// We use SplitMix as it is reputed to have a strong distribution of values.
133+
const prng = this.#getSplitMix32PRNG(seed);
134+
for (let i = array.length - 1; i > 0; i--) {
135+
const j = Math.floor(prng() * (i + 1));
136+
[array[i], array[j]] = [array[j], array[i]];
137+
}
138+
}
139+
140+
// SplitMix32 is a splittable pseudorandom number generator (PRNG).
141+
// License: MIT (https://github.com/attilabuti/SimplexNoise)
142+
#getSplitMix32PRNG(a) {
143+
return () => {
144+
a |= 0;
145+
a = (a + 0x9e3779b9) | 0;
146+
var t = a ^ (a >>> 16);
147+
t = Math.imul(t, 0x21f0aaad);
148+
t = t ^ (t >>> 15);
149+
t = Math.imul(t, 0x735a2d97);
150+
return ((t = t ^ (t >>> 15)) >>> 0) / 4294967296;
151+
};
152+
}
153+
154+
#restoreReasonsOrdering() {
155+
this.#reasonOptions[0].parentNode.append(...this.#reasonOptions);
156+
}
157+
105158
static CHOOSE_A_REASON_OPT_ID = "report-broken-site-popup-reason-choose";
106159

107160
get chooseAReasonOption() {
@@ -119,6 +172,19 @@ class ViewState {
119172
this.showOrHideReasonValidationMessage(false);
120173
}
121174

175+
ensureReasonOrderingMatchesPref() {
176+
const randomizeReasons =
177+
this.#doc.ownerGlobal.ReportBrokenSite.randomizeReasons;
178+
if (randomizeReasons != this.#randomizeReasons) {
179+
if (randomizeReasons) {
180+
this.#randomizeReasonsOrdering();
181+
} else {
182+
this.#restoreReasonsOrdering();
183+
}
184+
this.#randomizeReasons = randomizeReasons;
185+
}
186+
}
187+
122188
get isURLValid() {
123189
return this.urlInput.checkValidity();
124190
}
@@ -245,6 +311,8 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
245311
1: "optional",
246312
2: "required",
247313
};
314+
static REASON_RANDOMIZED_PREF =
315+
"ui.new-webcompat-reporter.reason-dropdown.randomized";
248316
static SEND_MORE_INFO_PREF = "ui.new-webcompat-reporter.send-more-info-link";
249317
static NEW_REPORT_ENDPOINT_PREF =
250318
"ui.new-webcompat-reporter.new-report-endpoint";
@@ -260,6 +328,7 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
260328

261329
#reasonEnabled = false;
262330
#reasonIsOptional = true;
331+
#randomizeReasons = false;
263332
#descriptionIsOptional = true;
264333
#sendMoreInfoEnabled = true;
265334

@@ -271,6 +340,10 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
271340
return this.#reasonIsOptional;
272341
}
273342

343+
get randomizeReasons() {
344+
return this.#randomizeReasons;
345+
}
346+
274347
get descriptionIsOptional() {
275348
return this.#descriptionIsOptional;
276349
}
@@ -279,6 +352,7 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
279352
for (const [name, [pref, dflt]] of Object.entries({
280353
dataReportingPref: [ReportBrokenSite.DATAREPORTING_PREF, false],
281354
reasonPref: [ReportBrokenSite.REASON_PREF, 0],
355+
reasonRandomizedPref: [ReportBrokenSite.REASON_RANDOMIZED_PREF, false],
282356
sendMoreInfoPref: [ReportBrokenSite.SEND_MORE_INFO_PREF, false],
283357
newReportEndpointPref: [
284358
ReportBrokenSite.NEW_REPORT_ENDPOINT_PREF,
@@ -440,32 +514,8 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
440514

441515
this.#sendMoreInfoEnabled = this.sendMoreInfoPref;
442516
this.#newReportEndpoint = this.newReportEndpointPref;
443-
}
444517

445-
#random(seed) {
446-
let x = Math.sin(seed) * 10000;
447-
return x - Math.floor(x);
448-
}
449-
450-
#shuffleArray(array) {
451-
const seed = Math.round(new Date().getTime());
452-
for (let i = array.length - 1; i > 0; i--) {
453-
const j = Math.floor(this.#random(seed) * (i + 1));
454-
[array[i], array[j]] = [array[j], array[i]];
455-
}
456-
}
457-
458-
#randomizeDropdownItems(dropdown) {
459-
if (!dropdown) {
460-
return;
461-
}
462-
463-
// Leave the first option ("choose reason") at the start
464-
const items = Array.from(
465-
dropdown.querySelectorAll(`menuitem:not(:first-of-type)`)
466-
);
467-
this.#shuffleArray(items);
468-
items[0].parentNode.append(...items);
518+
this.#randomizeReasons = this.reasonRandomizedPref;
469519
}
470520

471521
#initMainView(state) {
@@ -499,8 +549,6 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
499549
state.showOrHideReasonValidationMessage();
500550
});
501551

502-
this.#randomizeDropdownItems(reasonDropdown);
503-
504552
const menupopup = reasonDropdown.querySelector("menupopup");
505553
const onDropDownShowOrHide = ({ type }) => {
506554
// Hide "choose a reason" while the user has the reason dropdown open
@@ -542,6 +590,8 @@ export var ReportBrokenSite = new (class ReportBrokenSite {
542590

543591
state.reasonInput.hidden = !this.#reasonEnabled;
544592

593+
state.ensureReasonOrderingMatchesPref();
594+
545595
state.reasonLabelRequired.hidden =
546596
!this.#reasonEnabled || this.#reasonIsOptional;
547597
state.reasonLabelOptional.hidden =

browser/components/reportbrokensite/test/browser/browser_reason_dropdown.js

Lines changed: 77 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,30 +67,92 @@ add_task(async function testReasonDropdown() {
6767
);
6868
});
6969

70-
async function getListItems() {
71-
const win = await BrowserTestUtils.openNewBrowserWindow();
72-
const rbs = new ReportBrokenSiteHelper(AppMenu(win));
70+
async function getListItems(rbs) {
7371
const items = Array.from(rbs.reasonInput.querySelectorAll("menuitem")).map(
7472
i => i.id.replace("report-broken-site-popup-reason-", "")
7573
);
7674
Assert.equal(items[0], "choose", "First option is always 'choose'");
77-
await BrowserTestUtils.closeWindow(win);
78-
return items;
75+
return items.join(",");
7976
}
8077

8178
add_task(async function testReasonDropdownRandomized() {
8279
ensureReportBrokenSitePreffedOn();
8380
ensureReasonOptional();
8481

85-
let isRandomized = false;
86-
const list1 = await getListItems();
87-
for (let attempt = 0; attempt < 100; ++attempt) {
88-
// try up to 100 times
89-
const list = await getListItems();
90-
if (!areObjectsEqual(list, list1)) {
91-
isRandomized = true;
92-
break;
82+
const USER_ID_PREF = "app.normandy.user_id";
83+
const RANDOMIZE_PREF = "ui.new-webcompat-reporter.reason-dropdown.randomized";
84+
85+
const origNormandyUserID = Services.prefs.getCharPref(
86+
USER_ID_PREF,
87+
undefined
88+
);
89+
90+
await BrowserTestUtils.withNewTab(
91+
REPORTABLE_PAGE_URL,
92+
async function (browser) {
93+
// confirm that the default order is initially used
94+
Services.prefs.setBoolPref(RANDOMIZE_PREF, false);
95+
const rbs = await AppMenu().openReportBrokenSite();
96+
const defaultOrder = [
97+
"choose",
98+
"slow",
99+
"media",
100+
"content",
101+
"account",
102+
"adblockers",
103+
"other",
104+
];
105+
Assert.deepEqual(
106+
await getListItems(rbs),
107+
defaultOrder,
108+
"non-random order is correct"
109+
);
110+
111+
// confirm that a random order happens per user
112+
let randomOrder;
113+
let isRandomized = false;
114+
Services.prefs.setBoolPref(RANDOMIZE_PREF, true);
115+
116+
// This becomes ClientEnvironment.randomizationId, which we can set to
117+
// any value which results in a different order from the default ordering.
118+
Services.prefs.setCharPref("app.normandy.user_id", "dummy");
119+
120+
// clicking cancel triggers a reset, which is when the randomization
121+
// logic is called. so we must click cancel after pref-changes here.
122+
rbs.clickCancel();
123+
await AppMenu().openReportBrokenSite();
124+
randomOrder = await getListItems(rbs);
125+
Assert.ok(
126+
randomOrder != defaultOrder,
127+
"options are randomized with pref on"
128+
);
129+
130+
// confirm that the order doesn't change per user
131+
isRandomized = false;
132+
for (let attempt = 0; attempt < 5; ++attempt) {
133+
rbs.clickCancel();
134+
await AppMenu().openReportBrokenSite();
135+
const order = await getListItems(rbs);
136+
137+
if (order != randomOrder) {
138+
isRandomized = true;
139+
break;
140+
}
141+
}
142+
Assert.ok(!isRandomized, "options keep the same order per user");
143+
144+
// confirm that the order reverts to the default if pref flipped to false
145+
Services.prefs.setBoolPref(RANDOMIZE_PREF, false);
146+
rbs.clickCancel();
147+
await AppMenu().openReportBrokenSite();
148+
Assert.deepEqual(
149+
defaultOrder,
150+
await getListItems(rbs),
151+
"reverts to non-random order correctly"
152+
);
153+
rbs.clickCancel();
93154
}
94-
}
95-
Assert.ok(isRandomized, "Downdown options are randomized");
155+
);
156+
157+
Services.prefs.setCharPref(USER_ID_PREF, origNormandyUserID);
96158
});

0 commit comments

Comments
 (0)