Skip to content

[FIX] web_dark_mode: always use dark mode on new tabs#3458

Open
len-foss wants to merge 1 commit intoOCA:18.0from
lambdao-dev:18.0-fixdrkmd
Open

[FIX] web_dark_mode: always use dark mode on new tabs#3458
len-foss wants to merge 1 commit intoOCA:18.0from
lambdao-dev:18.0-fixdrkmd

Conversation

@len-foss
Copy link
Contributor

Suppose you have your Odoo running at http://localhost:8069 (or https://domain.tld, doesn't matter)
Set your scheme to dark mode. Odoo switches to dark mode.
Open a new tab http://localhost:8069, it uses the light theme. Refresh the tab, it now uses the dark mode.
If you open your tab at http://localhost:8069/odoo (or any non-empty backend path) it would properly use dark mode. However, browser auto-complete defaults to the root address.

@len-foss
Copy link
Contributor Author

@ljmnoonan could you confirm this? I suspect it also affects #3410 ?

As a side-note, the tests fail but they are testing how the code was implemented, not whether it works or not.

@ljmnoonan
Copy link
Member

@len-foss I'll take a look later, but just to verify, are you using the dark mode toggle or the device dependant dark mode? If it is the latter, then I know it is broken and has been that way since the beginning. Basically, js was only checking devices preference if the color_scheme cookie didn't exist. I fixed in 5ef5802

@len-foss
Copy link
Contributor Author

@ljmnoonan no, I'm talking about the regular dark mode toggle. The bug is that in the redirect dispatch the template is rendered before the color scheme is applied. If my fix is indeed the proper solution to the bug, you would probably have to rewrite just a few lines of your fix.

@ljmnoonan
Copy link
Member

@len-foss , ok, I got some time to test it out and I see what the problem is. This was fixed in odoo 19.0 by odoo/odoo@2120e4c which

... sets the color_scheme in the QWeb rendering context
instead of directly access the cookie

so it does not affect #3410.

Your fix does indeed work, but the choice here is do we want to use your solution to patch the old logic, or effectively backport the odoo changes into this model (which would involve inheriting a very low priority view)? If you think the latter option is better I would also be able to backport 993e2d3 which fixes device dependent dark mode and implements server side asset bundling based on the Sec-CH-Prefers-Color-Scheme hint from chromium browsers.
I also realized that the reason I have never noticed this is that I normally have website installed, which prevents root domain from redirecting to a backend path, so this bug is limited to installations that are using the backend only.

@len-foss
Copy link
Contributor Author

@ljmnoonan sorry about the delay, and thank you for your reply. The backport is probably the best way? If only for ease of maintenance. I'm not sure of whether it requires much adaptations or not. Can you prepare a branch? If there's some problem to fix with it I can fix it, test it on my end and give a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants