Bug 2069172 - Record appearance and native-theme changes from the theme picker. r=omc-reviewers,mstriemer,jprickett
Record each appearance and native-theme change with its selected value, source, and layout. Record direct updates locally. Record remote updates after the parent applies them. Keep theme selection in ThemesList and preserve its per-source attribution. Differential Revision: https://phabricator.services.mozilla.com/D324462
This commit is contained in:
committed by
mstriemer@mozilla.com
parent
e638419e1b
commit
85c4f8447e
@@ -19,6 +19,21 @@ const PREF_ACTIVE_THEME_ID = "extensions.activeThemeID";
|
||||
* and updates via AddonManager and prefs.
|
||||
*/
|
||||
export class ThemePickerParent extends JSWindowActorParent {
|
||||
themesManagers = new Map();
|
||||
|
||||
async getThemesManager(installSource) {
|
||||
let managerPromise = this.themesManagers.get(installSource);
|
||||
if (!managerPromise) {
|
||||
managerPromise = lazy.getThemesList({ installSource }).catch(error => {
|
||||
this.themesManagers.delete(installSource);
|
||||
throw error;
|
||||
});
|
||||
this.themesManagers.set(installSource, managerPromise);
|
||||
}
|
||||
|
||||
return managerPromise;
|
||||
}
|
||||
|
||||
async receiveMessage(message) {
|
||||
switch (message.name) {
|
||||
case "ThemePicker:GetInitialState":
|
||||
@@ -47,11 +62,8 @@ export class ThemePickerParent extends JSWindowActorParent {
|
||||
}
|
||||
|
||||
async getInitialState({ installSource, showInCompactLayout }) {
|
||||
if (!this.themesManager) {
|
||||
this.themesManager = await lazy.getThemesList({ installSource });
|
||||
}
|
||||
|
||||
const themes = this.themesManager.getThemesInfo({ showInCompactLayout });
|
||||
const themesManager = await this.getThemesManager(installSource);
|
||||
const themes = themesManager.getThemesInfo({ showInCompactLayout });
|
||||
const { activeThemeId } = this.getActiveThemeId();
|
||||
const { nativeTheme } = this.getNativeTheme();
|
||||
const appearance = this.getAppearanceFromPref();
|
||||
@@ -71,12 +83,13 @@ export class ThemePickerParent extends JSWindowActorParent {
|
||||
};
|
||||
}
|
||||
|
||||
async updateTheme({ themeId, layout }) {
|
||||
await this.themesManager.updateThemeState(themeId, true, { layout });
|
||||
async updateTheme({ themeId, installsource, layout }) {
|
||||
const themesManager = await this.getThemesManager(installsource);
|
||||
await themesManager.updateThemeState(themeId, true, { layout });
|
||||
return this.getActiveThemeId();
|
||||
}
|
||||
|
||||
async updateAppearance({ appearance }) {
|
||||
async updateAppearance({ appearance, installsource, layout }) {
|
||||
if (appearance === "device") {
|
||||
Services.prefs.clearUserPref(PREF_SYSTEM_USES_DARK);
|
||||
} else {
|
||||
@@ -86,13 +99,29 @@ export class ThemePickerParent extends JSWindowActorParent {
|
||||
);
|
||||
}
|
||||
|
||||
return this.getAppearance();
|
||||
const result = this.getAppearance();
|
||||
Glean.themePicker.change.record({
|
||||
source: installsource,
|
||||
layout,
|
||||
property: "appearance",
|
||||
appearance: result.appearance,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
async updateNativeTheme({ nativeTheme }) {
|
||||
async updateNativeTheme({ nativeTheme, installsource, layout }) {
|
||||
Services.prefs.setBoolPref(PREF_NATIVE_THEME, nativeTheme);
|
||||
|
||||
return this.getNativeTheme();
|
||||
const result = this.getNativeTheme();
|
||||
Glean.themePicker.change.record({
|
||||
source: installsource,
|
||||
layout,
|
||||
property: "nativeTheme",
|
||||
native_theme: result.nativeTheme,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
getActiveThemeId() {
|
||||
|
||||
+105
-5
@@ -4,6 +4,10 @@ const { AboutWelcomeDefaults } = ChromeUtils.importESModule(
|
||||
"resource:///modules/aboutwelcome/AboutWelcomeDefaults.sys.mjs"
|
||||
);
|
||||
|
||||
const { ThemePickerParent } = ChromeUtils.importESModule(
|
||||
"resource:///actors/ThemePickerParent.sys.mjs"
|
||||
);
|
||||
|
||||
function getThemePickerScreen() {
|
||||
const { screens } = AboutWelcomeDefaults.getDefaults();
|
||||
return screens.find(screen => screen.id === "AW_THEME_PICKER");
|
||||
@@ -11,7 +15,11 @@ function getThemePickerScreen() {
|
||||
|
||||
add_task(async function test_aboutwelcome_theme_picker_screen_displays() {
|
||||
// AW_THEME_PICKER is targeted on browser.nova.enabled.
|
||||
await pushPrefs(["browser.nova.enabled", true]);
|
||||
await pushPrefs(
|
||||
["browser.nova.enabled", true],
|
||||
["ui.systemUsesDarkTheme", -1],
|
||||
["browser.theme.native-theme", false]
|
||||
);
|
||||
await setAboutWelcomeMultiStage(JSON.stringify([getThemePickerScreen()]));
|
||||
|
||||
Services.fog.testResetFOG();
|
||||
@@ -26,17 +34,18 @@ add_task(async function test_aboutwelcome_theme_picker_screen_displays() {
|
||||
);
|
||||
|
||||
await SpecialPowers.spawn(browser, [], async () => {
|
||||
const themePicker = await ContentTaskUtils.waitForCondition(
|
||||
const renderedThemePicker = await ContentTaskUtils.waitForCondition(
|
||||
() => content.document.querySelector("theme-picker"),
|
||||
"theme-picker element should be present"
|
||||
);
|
||||
|
||||
await themePicker.updateComplete;
|
||||
await renderedThemePicker.updateComplete;
|
||||
|
||||
await ContentTaskUtils.waitForCondition(
|
||||
() =>
|
||||
!!themePicker.shadowRoot.querySelectorAll("moz-visual-picker-item")
|
||||
.length,
|
||||
!!renderedThemePicker.shadowRoot.querySelectorAll(
|
||||
"moz-visual-picker-item"
|
||||
).length,
|
||||
"theme-picker should render at least one theme button"
|
||||
);
|
||||
});
|
||||
@@ -59,6 +68,97 @@ add_task(async function test_aboutwelcome_theme_picker_screen_displays() {
|
||||
"The full picker layout should be recorded"
|
||||
);
|
||||
|
||||
Services.fog.testResetFOG();
|
||||
|
||||
const { source, layout, nativeThemeChanged } = await SpecialPowers.spawn(
|
||||
browser,
|
||||
[],
|
||||
async () => {
|
||||
const themePicker = content.document.querySelector("theme-picker");
|
||||
const picker = themePicker.wrappedJSObject;
|
||||
const EventUtils = ContentTaskUtils.getEventUtils(content);
|
||||
|
||||
const appearanceButton = themePicker.shadowRoot.querySelector(
|
||||
'moz-segmented-control-item[value="dark"]'
|
||||
);
|
||||
EventUtils.synthesizeMouseAtCenter(appearanceButton, {}, content);
|
||||
await ContentTaskUtils.waitForCondition(
|
||||
() => picker.appearance === "dark",
|
||||
"The rendered theme picker should update to dark appearance"
|
||||
);
|
||||
|
||||
const nativeThemeCheckbox =
|
||||
themePicker.shadowRoot.querySelector("moz-checkbox");
|
||||
if (nativeThemeCheckbox) {
|
||||
EventUtils.synthesizeMouseAtCenter(nativeThemeCheckbox, {}, content);
|
||||
await ContentTaskUtils.waitForCondition(
|
||||
() => picker.nativeTheme,
|
||||
"The rendered theme picker should enable the native theme"
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
source: themePicker.getAttribute("installsource"),
|
||||
layout: picker.layout,
|
||||
nativeThemeChanged: !!nativeThemeCheckbox,
|
||||
};
|
||||
}
|
||||
);
|
||||
|
||||
await Services.fog.testFlushAllChildren();
|
||||
|
||||
const expectedChangeExtras = [
|
||||
{
|
||||
source,
|
||||
layout,
|
||||
property: "appearance",
|
||||
appearance: "dark",
|
||||
},
|
||||
];
|
||||
if (nativeThemeChanged) {
|
||||
expectedChangeExtras.push({
|
||||
source,
|
||||
layout,
|
||||
property: "nativeTheme",
|
||||
native_theme: "true",
|
||||
});
|
||||
}
|
||||
|
||||
const changeEvents = Glean.themePicker.change.testGetValue();
|
||||
Assert.deepEqual(
|
||||
changeEvents?.map(event => event.extra),
|
||||
expectedChangeExtras,
|
||||
"Clicked picker changes should record their telemetry extras"
|
||||
);
|
||||
|
||||
await cleanup();
|
||||
await popPrefs();
|
||||
});
|
||||
|
||||
add_task(async function test_theme_picker_parent_retries_failed_manager_load() {
|
||||
const parent = new ThemePickerParent();
|
||||
|
||||
async function getManagerError() {
|
||||
let error;
|
||||
try {
|
||||
await parent.getThemesManager();
|
||||
} catch (caughtError) {
|
||||
error = caughtError;
|
||||
}
|
||||
Assert.stringContains(
|
||||
error?.message,
|
||||
"getThemesList installSource option is mandatory",
|
||||
"The manager load should fail for a missing install source"
|
||||
);
|
||||
return error;
|
||||
}
|
||||
|
||||
const firstError = await getManagerError();
|
||||
const secondError = await getManagerError();
|
||||
|
||||
Assert.notEqual(
|
||||
firstError,
|
||||
secondError,
|
||||
"A failed manager load should be retried"
|
||||
);
|
||||
});
|
||||
|
||||
@@ -2267,21 +2267,18 @@ theme_picker:
|
||||
property:
|
||||
type: string
|
||||
description: >
|
||||
Which setting changed. Only `"theme"` is recorded today.
|
||||
`"appearance"` and `"nativeTheme"` are declared ahead of the
|
||||
appearance and native theme instrumentation.
|
||||
Which setting changed: `"theme"`, `"appearance"`, or `"nativeTheme"`.
|
||||
theme_id:
|
||||
type: string
|
||||
description: >
|
||||
The addon ID of the theme selected. Set on every recorded
|
||||
event, since `property` is always `"theme"` today.
|
||||
The addon ID of the theme selected. Set when `property` is `"theme"`.
|
||||
appearance:
|
||||
type: string
|
||||
description: >
|
||||
The appearance mode selected, one of `"light"`, `"dark"`, or
|
||||
`"device"`. Not recorded until `property` can be `"appearance"`.
|
||||
`"device"`. Set when `property` is `"appearance"`.
|
||||
native_theme:
|
||||
type: boolean
|
||||
description: >
|
||||
Whether native theme styling was enabled. Not recorded until
|
||||
`property` can be `"nativeTheme"`.
|
||||
Whether native theme styling was enabled. Set when `property` is
|
||||
`"nativeTheme"`.
|
||||
|
||||
@@ -432,8 +432,12 @@
|
||||
host.dispatchEvent(new CustomEvent("themepickershown"));
|
||||
SimpleTest.isDeeply(
|
||||
details.get("ThemePickerUpdateTheme"),
|
||||
{ themeId: "nova-sun@mozilla.org", layout: "compact" },
|
||||
"Theme updates carry the host layout."
|
||||
{
|
||||
themeId: "nova-sun@mozilla.org",
|
||||
installsource: "test-source",
|
||||
layout: "compact",
|
||||
},
|
||||
"Theme updates carry the host source and layout."
|
||||
);
|
||||
SimpleTest.isDeeply(
|
||||
details.get("ThemePickerUpdateAppearance"),
|
||||
@@ -523,6 +527,97 @@
|
||||
);
|
||||
});
|
||||
|
||||
add_task(async function testDirectControllerRecordsChangesForEveryCall() {
|
||||
useController(ThemePickerDirectController);
|
||||
let picker = await renderThemePicker({
|
||||
layout: "full",
|
||||
installsource: "test-source",
|
||||
});
|
||||
await Services.fog.testFlushAllChildren();
|
||||
Services.fog.testResetFOG();
|
||||
|
||||
let updateThemeStatePromise;
|
||||
let { themesManager } = picker.controller;
|
||||
let updateThemeState =
|
||||
themesManager.updateThemeState.bind(themesManager);
|
||||
themesManager.updateThemeState = (...args) => {
|
||||
updateThemeStatePromise = updateThemeState(...args);
|
||||
return updateThemeStatePromise;
|
||||
};
|
||||
|
||||
picker.dispatchChange("appearance", "dark");
|
||||
picker.dispatchChange("appearance", "dark");
|
||||
picker.dispatchChange("appearance", "light");
|
||||
picker.dispatchChange("appearance", "device");
|
||||
picker.dispatchChange("nativeTheme", true);
|
||||
picker.dispatchChange("nativeTheme", true);
|
||||
picker.dispatchChange("nativeTheme", false);
|
||||
picker.dispatchChange("theme", DEFAULT_THEME_ID);
|
||||
await updateThemeStatePromise;
|
||||
|
||||
await Services.fog.testFlushAllChildren();
|
||||
let events = Glean.themePicker.change.testGetValue();
|
||||
is(
|
||||
events?.length,
|
||||
8,
|
||||
"theme_picker.change is recorded for every call."
|
||||
);
|
||||
SimpleTest.isDeeply(
|
||||
events?.map(event => event.extra),
|
||||
[
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "appearance",
|
||||
appearance: "dark",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "appearance",
|
||||
appearance: "dark",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "appearance",
|
||||
appearance: "light",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "appearance",
|
||||
appearance: "device",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "nativeTheme",
|
||||
native_theme: "true",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "nativeTheme",
|
||||
native_theme: "true",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "nativeTheme",
|
||||
native_theme: "false",
|
||||
},
|
||||
{
|
||||
source: "test-source",
|
||||
layout: "full",
|
||||
property: "theme",
|
||||
theme_id: DEFAULT_THEME_ID,
|
||||
},
|
||||
],
|
||||
"Change events retain every call's source, layout, property, and value."
|
||||
);
|
||||
});
|
||||
|
||||
// Selecting a theme delegates to the themes manager and reflects the
|
||||
// resulting active theme. The manager's install/enable behavior (which
|
||||
// talks to AddonManager) is covered by FirefoxThemesList tests, so it is
|
||||
|
||||
@@ -32,17 +32,6 @@ export class ThemePickerDirectController {
|
||||
constructor(host) {
|
||||
this.host = host;
|
||||
this.host.addController(this);
|
||||
lazy
|
||||
.getThemesList({
|
||||
installSource: this.host.getAttribute("installsource") || "unknown",
|
||||
})
|
||||
.then(tm => {
|
||||
this.themesManager = tm;
|
||||
this.host.themes = tm.getThemesInfo({
|
||||
showInCompactLayout: this.host.layout === "compact",
|
||||
});
|
||||
this.updateHost();
|
||||
});
|
||||
this.lazy = XPCOMUtils.declareLazy({
|
||||
activeThemeId: {
|
||||
pref: PREF_ACTIVE_THEME_ID,
|
||||
@@ -77,6 +66,17 @@ export class ThemePickerDirectController {
|
||||
}
|
||||
|
||||
hostConnected() {
|
||||
this.themesManagerPromise ??= lazy
|
||||
.getThemesList({
|
||||
installSource: this.host.getAttribute("installsource") || "unknown",
|
||||
})
|
||||
.then(tm => {
|
||||
this.themesManager = tm;
|
||||
this.host.themes = tm.getThemesInfo({
|
||||
showInCompactLayout: this.host.layout === "compact",
|
||||
});
|
||||
this.updateHost();
|
||||
});
|
||||
Services.obs.addObserver(this.updateHost, "look-and-feel-changed");
|
||||
this.updateHost();
|
||||
}
|
||||
@@ -102,9 +102,21 @@ export class ThemePickerDirectController {
|
||||
value == "light" ? 0 : 1
|
||||
);
|
||||
}
|
||||
Glean.themePicker.change.record({
|
||||
source: this.host.getAttribute("installsource") || "unknown",
|
||||
layout: this.host.layout || "unknown",
|
||||
property,
|
||||
appearance: String(value),
|
||||
});
|
||||
break;
|
||||
case "nativeTheme":
|
||||
Services.prefs.setBoolPref(PREF_NATIVE_THEME, Boolean(value));
|
||||
Glean.themePicker.change.record({
|
||||
source: this.host.getAttribute("installsource") || "unknown",
|
||||
layout: this.host.layout || "unknown",
|
||||
property,
|
||||
native_theme: Boolean(value),
|
||||
});
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,6 +123,7 @@ export class ThemePickerRemoteController {
|
||||
case "theme":
|
||||
this.dispatchActorEvent("ThemePickerUpdateTheme", {
|
||||
themeId: String(value),
|
||||
installsource: this.installSource,
|
||||
layout: this.host.layout,
|
||||
});
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user