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

dark theme mismatch for some UI items on older OSes #4637

Closed
LaurenWags opened this issue May 29, 2019 · 9 comments · Fixed by brave/brave-core#2905
Closed

dark theme mismatch for some UI items on older OSes #4637

LaurenWags opened this issue May 29, 2019 · 9 comments · Fixed by brave/brave-core#2905

Comments

@LaurenWags
Copy link
Member

LaurenWags commented May 29, 2019

Description

On older OS such as High Sierra, Chromium items (web ui, menus) do not go dark to match selected theme but Brave specific items (shields, toolbar) do.

Newer OSes (such as Mojave) do not have this problem.

Test plan / Steps to Reproduce

  1. On a machine with an older OS, such as High Sierra, change theme to dark in settings.
  2. Toolbar becomes dark as expected.
  3. Visit a page and open shields. Verify shields are dark as well.
  4. Open hamburger menu. It is not dark.
  5. Visit a page like brave://bookmarks, history, etc.
  6. Page is not dark but toolbar is.

Actual result:

Hamburger menu and page backgrounds do not match dark theme.
Screen Shot 2019-05-29 at 12 15 49 PM

Screen Shot 2019-05-29 at 12 07 00 PM

Expected result:

Hamburger menu and page backgrounds should match dark theme as shields and toolbar do.
Screen Shot 2019-05-29 at 12 15 43 PM

Reproduces how often:

easily

Brave version (brave://version info)

Brave 0.65.113 Chromium: 75.0.3770.38 (Official Build) beta(64-bit)
Revision 3860105745f2b12537da9e9f048f14c3f52ba970-refs/branch-heads/3770@{#618}
OS Mac OS X

Version/Channel Information:

  • Can you reproduce this issue with the current release? yes
  • Can you reproduce this issue with the beta channel? yes
  • Can you reproduce this issue with the dev channel? yes
  • Can you reproduce this issue with the nightly channel? yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? n/a
  • Does the issue resolve itself when disabling Brave Rewards? n/a
  • Is the issue reproducible on the latest version of Chrome? per discussion with @petemill Chrome doesn't have dark theme for older OSes

Miscellaneous Information:

Some screenshots of how Mojave behaves can be found here: #3965 (comment)

@simonhong
Copy link
Member

I think this wouldn't be fixed because upstream chromium only support dark mode to native ui(ex menu) or webui for Mojave.

@bsclifton bsclifton added the Chromium/waiting upstream Issue is in Chromium; we'll likely wait for the fix label May 29, 2019
@bsclifton
Copy link
Member

Given what @simonhong mentioned, I tagged with a waiting upstream label (as it doesn't make sense to go against Chromium for this). If it never gets addressed, we can likely close with wontfix

@LaurenWags
Copy link
Member Author

@simonhong
Copy link
Member

@LaurenWags Yup, this is expected behavior for current upstream implementation.
Dark mode is disabled by default on linux platform like old MacOS version.
I think we should support if user explicitly choose light or dark instead of Same as XXX.

@simonhong
Copy link
Member

@LaurenWags I think this also can be happened on older Windows like Win7 and Win8 because they don't support dark mode. Can you check this on Win7 and Win8 also?

@LaurenWags
Copy link
Member Author

@simonhong I don't have Win 8, but I can confirm this also happens for Win 7.

Brave 0.64.77 Chromium: 74.0.3729.169 (Official Build) (64-bit)
Revision 78e4f8db3ce38f6c26cf56eed7ae9b331fc67ada-refs/branch-heads/3729@{#1013}
OS Windows 7 Service Pack 1 Build 7601.24334

Screen Shot 2019-06-04 at 3 36 26 PM

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Jun 7, 2019
@Brave-Matt
Copy link

I can reproduce this on Win10 in both Nightly and Release builds. I have another user here confirming Laurens initial report:
https://www.reddit.com/r/brave_browser/comments/bxk3by/release_channel_update_v065118_is_live/eq8gxno?utm_source=share&utm_medium=web2xhttps://www.reddit.com/r/brave_browser/comments/bxk3by/release_channel_update_v065118_is_live/eq8gxno?utm_source=share&utm_medium=web2x

@simonhong
Copy link
Member

simonhong commented Jun 7, 2019

@Brave-Matt Thanks for checking this on Nightly on Windows.

In C75 (75.0.3770.80), dark mode support on Windows is disabled. (It was supported latest C74 by field trial).
and latest chromium enabled it. There is no kDarkMode feature in https://cs.chromium.org/chromium/src/ui/base/ui_base_features.h

#if defined(OS_MACOSX)
const base::Feature kDarkMode = {"DarkMode", base::FEATURE_ENABLED_BY_DEFAULT};
#else
const base::Feature kDarkMode = {"DarkMode", base::FEATURE_DISABLED_BY_DEFAULT};
#endif

I assume(not sure) that chrome team internally picked https://chromium-review.googlesource.com/c/chromium/src/+/1597930 because chrome 75 stable support dark mode on Win10. -> It was enabled by field trial.

@bbondy @bsclifton @mkarolin @rebron Should we also pick that?
If not, we don't have Same as Windows mode in Win10 with current Nightly(0.68).

Sorry for the noise. Nightly is different thing with this issue. Will create new issue.

@btlechowski
Copy link

btlechowski commented Aug 27, 2019

Dark mode works awesome! Great work!

Verification passed on

Brave 0.69.117 Chromium: 76.0.3809.100 (Official Build) beta (64-bit)
Revision ed9d447d30203dc5069e540f05079e493fc1c132-refs/branch-heads/3809@{#990}
OS Ubuntu 18.04 LTS

Verified steps from the description.
Also verified Light mode.

image
image
image
image
image

Verified passed with

Brave 0.69.119 Chromium: 76.0.3809.132 (Official Build) beta (64-bit)
Revision fd1acc410994a7a68ac25bc77513d443f3130860-refs/branch-heads/3809@{#1035}
OS Mac OS X

Verification passed on

Brave 0.69.119 Chromium: 76.0.3809.132 (Official Build) beta (64-bit)
Revision fd1acc410994a7a68ac25bc77513d443f3130860-refs/branch-heads/3809@{#1035}
OS Windows 7 Service Pack 1 (Build 7601.24494)

Verified steps from the description.
Also verified Light mode.

image
image
image
image
image

@rebron rebron changed the title Chromium items do not go dark to match theme but Brave specific items do dark theme mismatch for some UI items on older OSes Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants