Skip to content

Commit

Permalink
Keep all theme-updating logic together
Browse files Browse the repository at this point in the history
Prior to this PR, if the page is restored from the browser bfcache¹, we
call `switchToSavedTheme`. But `switchToSavedTheme` never looks at the
`use-system-theme` preference. Further, if it can't find a saved theme,
it will fall back to the default of "light".

For a user with cookies disabled² whose preferred color scheme is dark,
this means the theme will wobble back and forth between dark and light.
The sequence that occurs is,

1. The page is loaded. During a page load, we consult
   `use-system-theme`: as cookies are disabled, this preference is
   unset. The default is true.

   Because the default is true, we look at the preferred color scheme:
   for our example user, that's "dark". **The page theme is set to
   dark.** We'll attempt to store these preferences in localStorage, but
   fail due to cookies being disabled.

2. The user navigates through the docs. Subsequent page loads happen,
   and the same process in step 1 recurs. Previous pages are
   (potentially) put into the bfcache.

3. The user navigates backwards/forwards, causing a page in bfcache to
   be pulled out of cache. The `pageShow` event handler is triggered.
   However, this calls `switchToSavedTheme`: this doesn't consider the
   system theme, as noted above. Instead, it only looks for a saved
   theme. However, with cookies disabled, there is none. It defaults to
   light. **The page theme is set to light!** The user wonders why the
   dark theme is lost.

There are effectively two functions trying to determine and apply the
correct theme: `updateSystemTheme` and `switchToSavedTheme`. Thus, we
merge them into just one: `updateTheme`. This function contains all the
logic for determining the correct theme, and is called in all
circumstances where we need to set the theme:

* The initial page load
* If the browser preferred color scheme (i.e., light/dark mode) is
  changed
* If the page is restored from bfcache
* If the user updates the theme preferences (i.e., in `settings.js`)

Fixes #94250.

¹bfcache: https://web.dev/bfcache/ The bfcache is used to sleep a page,
if the user navigates away from it, and to restore it from cache if the
user returns to it.

²Note that the browser preference that enables/disables cookies really
controls many forms of storage. The same preference thus also affects
localStorage. (This is so a normal browser user doesn't need to
understand the distinction between "cookies" and "localStorage".)
  • Loading branch information
thanatos committed Jan 30, 2023
1 parent fa214c3 commit 727a1fd
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 46 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/html/static/js/settings.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Local js definitions:
/* global getSettingValue, getVirtualKey, updateLocalStorage, updateSystemTheme */
/* global getSettingValue, getVirtualKey, updateLocalStorage, updateTheme */
/* global addClass, removeClass, onEach, onEachLazy, blurHandler, elemIsInParent */
/* global MAIN_ID, getVar, getSettingsButton */

Expand All @@ -19,7 +19,7 @@
case "theme":
case "preferred-dark-theme":
case "preferred-light-theme":
updateSystemTheme();
updateTheme();
updateLightAndDark();
break;
case "line-numbers":
Expand Down
96 changes: 52 additions & 44 deletions src/librustdoc/html/static/js/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,63 +153,74 @@ function switchTheme(styleElem, mainStyleElem, newThemeName, saveTheme) {
}
}

const updateSystemTheme = (function() {
if (!window.matchMedia) {
// fallback to the CSS computed value
return () => {
const cssTheme = getComputedStyle(document.documentElement)
.getPropertyValue("content");

switchTheme(
window.currentTheme,
window.mainTheme,
JSON.parse(cssTheme) || "light",
true
);
const updateTheme = (function() {
/**
* Update the current theme to match whatever the current combination of
* * the preference for using the system theme
* (if this is the case, the value of preferred-light-theme, if the
* system theme is light, otherwise if dark, the value of
* preferred-dark-theme.)
* * the preferred theme
* … dictates that it should be.
*/
function updateTheme() {
const use = (theme, saveTheme) => {
switchTheme(window.currentTheme, window.mainTheme, theme, saveTheme);
};
}

// only listen to (prefers-color-scheme: dark) because light is the default
const mql = window.matchMedia("(prefers-color-scheme: dark)");

function handlePreferenceChange(mql) {
const use = theme => {
switchTheme(window.currentTheme, window.mainTheme, theme, true);
};
// maybe the user has disabled the setting in the meantime!
if (getSettingValue("use-system-theme") !== "false") {
const lightTheme = getSettingValue("preferred-light-theme") || "light";
const darkTheme = getSettingValue("preferred-dark-theme") || "dark";

if (mql.matches) {
use(darkTheme);
if (isDarkMode()) {
use(darkTheme, true);
} else {
// prefers a light theme, or has no preference
use(lightTheme);
use(lightTheme, true);
}
// note: we save the theme so that it doesn't suddenly change when
// the user disables "use-system-theme" and reloads the page or
// navigates to another page
} else {
use(getSettingValue("theme"));
use(getSettingValue("theme"), false);
}
}

mql.addListener(handlePreferenceChange);
// This is always updated below to a function () => bool.
let isDarkMode;

return () => {
handlePreferenceChange(mql);
};
})();
// Determine the function for isDarkMode, and if we have
// `window.matchMedia`, set up an event listener on the preferred color
// scheme.
//
// Otherwise, fall back to the prefers-color-scheme value CSS captured in
// the "content" property.
if (window.matchMedia) {
// only listen to (prefers-color-scheme: dark) because light is the default
const mql = window.matchMedia("(prefers-color-scheme: dark)");

function switchToSavedTheme() {
switchTheme(
window.currentTheme,
window.mainTheme,
getSettingValue("theme") || "light",
false
);
}
isDarkMode = () => mql.matches;

if (mql.addEventListener) {
mql.addEventListener("change", updateTheme);
} else {
// This is deprecated, see:
// https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/addListener
mql.addListener(updateTheme);
}
} else {
// fallback to the CSS computed value
const cssContent = getComputedStyle(document.documentElement)
.getPropertyValue("content");
// (Note: the double-quotes come from that this is a CSS value, which
// might be a length, string, etc.)
const cssColorScheme = cssContent || "\"light\"";
isDarkMode = () => (cssColorScheme === "\"dark\"");
}

return updateTheme;
})();

if (getSettingValue("use-system-theme") !== "false" && window.matchMedia) {
// update the preferred dark theme if the user is already using a dark theme
Expand All @@ -219,13 +230,10 @@ if (getSettingValue("use-system-theme") !== "false" && window.matchMedia) {
&& darkThemes.indexOf(localStoredTheme) >= 0) {
updateLocalStorage("preferred-dark-theme", localStoredTheme);
}

// call the function to initialize the theme at least once!
updateSystemTheme();
} else {
switchToSavedTheme();
}

updateTheme();

if (getSettingValue("source-sidebar-show") === "true") {
// At this point in page load, `document.body` is not available yet.
// Set a class on the `<html>` element instead.
Expand All @@ -243,6 +251,6 @@ if (getSettingValue("source-sidebar-show") === "true") {
// specifically when talking to a remote website with no caching.
window.addEventListener("pageshow", ev => {
if (ev.persisted) {
setTimeout(switchToSavedTheme, 0);
setTimeout(updateTheme, 0);
}
});

0 comments on commit 727a1fd

Please sign in to comment.