Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep all theme-updating logic together #107177

Merged
merged 3 commits into from
Jan 30, 2023

Commits on Jan 21, 2023

  1. This function appears to be unused

    The comment says that it is called from main.js, but there don't seem to
    be any references to it in main.js.
    
    A quick ripgrep says there are no references in all of librustdoc.
    thanatos committed Jan 21, 2023
    Configuration menu
    Copy the full SHA
    ee9e8cd View commit details
    Browse the repository at this point in the history
  2. Fix typo in comment

    thanatos committed Jan 21, 2023
    Configuration menu
    Copy the full SHA
    fa214c3 View commit details
    Browse the repository at this point in the history

Commits on Jan 30, 2023

  1. Keep all theme-updating logic together

    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 rust-lang#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".)
    thanatos committed Jan 30, 2023
    Configuration menu
    Copy the full SHA
    727a1fd View commit details
    Browse the repository at this point in the history