Bug 1731678 Comment 1 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Ah okay I think this is because [tabpanels](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/browser/base/content/tabbrowser.js#52-53) use [canvas colors](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#326-327), which are technically [HCM aware](fb12df827908b/servo/components/style/values/specified/color.rs#463-464) because they use presShell vals, but [not theme aware](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#463-464).

So we should either adapt canvas colors to consider light/dark theming, and then fallback to presShell colors, or we should change the tabpanels css to only override with the system colors if HCM is enabled.

I'm a little confused by the css to begin with, because it reads "if we're using system colors, set background color to canvas. otherwise, if FF HCM is always on (so, regardless of OS HCM), set background color to whatever is set from the colors dialog pref". I think something like this would make more sense: 

```
      // By default, tabpanels are styled by client theme. We handle
      // styling in the presence of FF high contrast mode here.
      if (Services.prefs.getIntPref("browser.display.document_color_use") == 2) {
        if (Services.prefs.getBoolPref("browser.display.use_system_colors")) {
          // If FF high contrast mode is on, and system colors are being used,
          // we should style tab panels with these colors instead of the OS
          // theme colors, since this represents a more specific user preference.
          this.tabpanels.style.backgroundColor = "-moz-default-background-color";
        } else {
          // Otherwise if HCM is on but system colors is off, use the value
          // set for background color in the colors dialog.
          this.tabpanels.style.backgroundColor = Services.prefs.getCharPref(
            "browser.display.background_color"
          );
        }
      }
```

This won't handle the case where FF HCM is set to "with high contrast themes" and OS HCM is enabled, but AFAICT there's no way for us to get that information in js. 
This is sort of an odd intersection of HCM / OS theming. I'm not convinced background color is the right thing to be using here.
Ah okay I think this is because [tabpanels](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/browser/base/content/tabbrowser.js#52-53) use [canvas colors](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#326-327), which are technically [HCM aware](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#467) because they use presShell vals, but [not theme aware](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#463-464).

So we should either adapt canvas colors to consider light/dark theming, and then fallback to presShell colors, or we should change the tabpanels css to only override with the system colors if HCM is enabled.

I'm a little confused by the css to begin with, because it reads "if we're using system colors, set background color to canvas. otherwise, if FF HCM is always on (so, regardless of OS HCM), set background color to whatever is set from the colors dialog pref". I think something like this would make more sense: 

```
      // By default, tabpanels are styled by client theme. We handle
      // styling in the presence of FF high contrast mode here.
      if (Services.prefs.getIntPref("browser.display.document_color_use") == 2) {
        if (Services.prefs.getBoolPref("browser.display.use_system_colors")) {
          // If FF high contrast mode is on, and system colors are being used,
          // we should style tab panels with these colors instead of the OS
          // theme colors, since this represents a more specific user preference.
          this.tabpanels.style.backgroundColor = "-moz-default-background-color";
        } else {
          // Otherwise if HCM is on but system colors is off, use the value
          // set for background color in the colors dialog.
          this.tabpanels.style.backgroundColor = Services.prefs.getCharPref(
            "browser.display.background_color"
          );
        }
      }
```

This won't handle the case where FF HCM is set to "with high contrast themes" and OS HCM is enabled, but AFAICT there's no way for us to get that information in js. 
This is sort of an odd intersection of HCM / OS theming. I'm not convinced background color is the right thing to be using here.
Ah okay I think this is because [tabpanels](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/browser/base/content/tabbrowser.js#52-53) use [canvas colors](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#326-327), which are technically [HCM aware](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#467) because they use presShell vals, but [not theme aware](https://searchfox.org/mozilla-central/rev/cf90d36979cc9c411cea50fadedfb12df827908b/servo/components/style/values/specified/color.rs#463-464).

So we should either adapt canvas colors to consider light/dark theming, and then fallback to presShell colors, or we should change the tabpanels css to only override with the system colors if HCM is enabled.

I'm a little confused by the css to begin with, because it reads "if we're using system colors, set background color to canvas. otherwise, if FF HCM is always on (so, regardless of OS HCM), set background color to whatever is set from the colors dialog pref". I think something like this would make more sense: 

```
      // By default, tabpanels are styled by client theme. We handle
      // styling in the presence of FF high contrast mode here.
      if (Services.prefs.getIntPref("browser.display.document_color_use") == 2) {
        if (Services.prefs.getBoolPref("browser.display.use_system_colors")) {
          // If FF high contrast mode is on, and system colors are being used,
          // we should style tab panels with these colors instead of the OS
          // theme colors, since this represents a more specific user preference.
          this.tabpanels.style.backgroundColor = "-moz-default-background-color";
        } else {
          // Otherwise if HCM is on but system colors is off, use the value
          // set for background color in the colors dialog.
          this.tabpanels.style.backgroundColor = Services.prefs.getCharPref(
            "browser.display.background_color"
          );
        }
      }
```

This won't handle the case where FF HCM is set to "with high contrast themes" and OS HCM is enabled, but AFAICT there's no way for us to get that information in js. 
This is sort of an odd intersection of HCM / OS theming. I'm not convinced background color on its own is the right thing to be using here.

Back to Bug 1731678 Comment 1