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 is rendering white text on a white background #1016

Closed
demeralde opened this issue Apr 21, 2023 · 9 comments
Closed

Dark theme is rendering white text on a white background #1016

demeralde opened this issue Apr 21, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@demeralde
Copy link

Describe the bug
The default dark theme is rendering white text on a white background, which is barley legible.

To Reproduce
Steps to reproduce the behavior:

  1. Open an EPUB that has one of the sections I've provided in the screenshot. Navigate to this section in the eBook
  2. Turn on the "Dark" theme

Expected behavior
The dark theme should always render text that's legible, and it shouldn't have a white background because it's a dark theme.

Screenshots
Screenshot from 2023-04-21 22-55-56

Version:

  • Foliate version: 2.6.4
  • OS/Distribution and version: Pop!_OS 22.04 LTS
  • Desktop environment: GNOME 42.5
  • Installation method: Pop!_Shop
@demeralde demeralde added the bug Something isn't working label Apr 21, 2023
@johnfactotum
Copy link
Owner

Duplicate of #265

@johnfactotum johnfactotum marked this as a duplicate of #265 Apr 22, 2023
@johnfactotum johnfactotum closed this as not planned Won't fix, can't repro, duplicate, stale Apr 22, 2023
@demeralde
Copy link
Author

@johnfactotum the solutions I saw there don't seem adequate.

The text should render correctly by default if the user selects the dark theme. There shouldn't be any manual tweaking required whatsoever to get it to work.

@johnfactotum
Copy link
Owner

johnfactotum commented Apr 25, 2023

That's not really possible.

it shouldn't have a white background

The reading system doesn't know whether it should have a white background or not. The background is set by the publisher, and it's impossible to know why they have set such a background, and it's impossible to say what color it should be instead if it's not correct.

There are ways for the publisher to fix this if they want. If they haven't done that, then the invert filter is provided as a workaround for the user.

@demeralde
Copy link
Author

@johnfactotum I've used the default Reader app on macOS, and IIRC the dark theme never had any issues before like this.

If the publisher made a mistake (which does happen), there's no way for the user to edit the eBook, plus it's not practical to do so. It's not their responsibility.

Why should the user have to manually trigger this? If the user selected "dark theme", that should be enough, the rest is the software's job. It's not their responsibility to configure the dark theme, it's the software's responsibility.

Therefore the app should automatically use some sort of workaround instead. The manual tweaking should only be an escape hatch if the automatic method doesn't work.

For example, if there's white text rendered on a white background, then just invert the text colour by default so it stands out. Just like you recommended, but do it by default.

If the user can come to sort of decision for how to manually tweak it as you suggested, then you can replicate the same logic in code and make it automatic. The same logic would probably apply to many books, and each book can be tested against this logic to make sure it works and adapts for all of them. Anything that can be done manually can be automated.

@johnfactotum
Copy link
Owner

I've used the default Reader app on macOS, and IIRC the dark theme never had any issues before like this.

If you're talking about Apple Books, I don't believe it inverts anything at all, by default or otherwise.

It does have a special attribute, __ibooks_internal_theme, that the publisher can use to detect dark mode, which Foliate also supports.

Therefore the app should automatically use some sort of workaround instead. The manual tweaking should only be an escape hatch if the automatic method doesn't work.

If it applies the invert filter by default, correctly formatted books would be rendered incorrectly by default.

If the user can come to sort of decision for how to manually tweak it as you suggested, then you can replicate the same logic in code and make it automatic. The same logic would probably apply to many books, and each book can be tested against this logic to make sure it works and adapts for all of them.

What logic would that be, exactly?

@demeralde
Copy link
Author

@johnfactotum I'm sure there's at least one way to detect if it needs to be inverted or not. If a human can determine it's illegible, then the same decision making process should be able to be broken down into programmable logic.

I'm not exactly sure what that solution is. All I'm saying is it must exist, because otherwise a human wouldn't be able to determine the text is illegible. The same logic a human uses can be written in code.

If for some reason that's really difficult, then maybe it could even be run through a machine learning script to determine whether to invert it, which is trained on many books.

Obviously at that point it might be a lot of effort to solve, and it may be a low priority item. But IMO it's necessary for a truly fluid user experience. Perhaps it can be done one day.

@johnfactotum
Copy link
Owner

Well, I guess a very simple heuristic, that can be done without querying the DOM and doesn't even require a CSS parser (which we don't currently have), would be: if the CSS does not contain the string color-scheme, and if it contains the strings #, rgb, or hsl, etc., then use the invert mode.

This doesn't account for images. A very simple heuristic for that might be: if the page contains small images (say something like less than 50px in either direction), then it's likely not a photo and should be inverted. This is pretty shaky, though.

Another approach would be to treat different image formats differently:

  • Ignore JPEGs. Although they might look incorrect, they would at least remain legible.
  • Check if PNGs have alpha. If so, use invert mode.
  • Not sure about SVG. Probably do something similar to the check for CSS above.

But even then I would be hesitant to make something like this the default behavior. By default it should try as much as possible to render the book as authored. User overrides should be something that needs to be enabled manually.

@johnfactotum johnfactotum reopened this Apr 28, 2023
@johnfactotum johnfactotum added enhancement New feature or request and removed bug Something isn't working labels Apr 28, 2023
@demeralde
Copy link
Author

demeralde commented Apr 29, 2023

@johnfactotum if someone is using the dark theme and the author didn't adapt the book to work with dark themes, then it's already not being used as authored. Therefore wouldn't it be best to just make this the default behaviour if someone is using the dark theme?

@johnfactotum
Copy link
Owner

the author didn't adapt the book to work with dark themes

It's not possible to know whether this is the case or not. It can only guess. If the book's stylesheet contains color-scheme: dark, prefers-color-scheme, __ibooks_internal_theme, and .calibre-viewer-dark-colors, then you can take it as evidence that it does work with dark mode. However, the absence of these does not necessarily mean that the book wasn't designed with dark mode in mind.

By making it disabled by default, it would signal that the invert filter is a hack, a poor workaround, that should be discouraged, and not the proper way of doing things. And it's easy enough to toggle the invert filter now that the option is available directly in the primary menu (rather than being hidden in submenus) in the gtk4 branch.

But I suppose, since the result, unlike some other kinds of overrides, wouldn't be that catastrophic if it goes wrong, and it would have a clear opt-out mechanism and is unlikely to contribute to the arms race between the user agent and the publisher, at the end of the day, taking all things into consideration, it would not be unreasonable to enable it by default.

johnfactotum added a commit that referenced this issue Nov 14, 2023
Fixes #1016.

The main idea is taken from Readium CSS and is basically what Apple
does, too. So there's not even a need to provide a way to opt out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants