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

build: __ibooks_internal_theme selector no longer working in compatibility.css #377

Closed
acabal opened this issue Dec 9, 2020 · 12 comments · Fixed by #379
Closed

build: __ibooks_internal_theme selector no longer working in compatibility.css #377

acabal opened this issue Dec 9, 2020 · 12 comments · Fixed by #379
Assignees

Comments

@acabal
Copy link
Member

acabal commented Dec 9, 2020

In compatibility.css we target iBooks in night mode to invert images. But sometime in mid-2020 this selector stopped working. We need to figure out what an alternative solution is, if there is one. If not, we should remove this CSS.

@robinwhittleton robinwhittleton self-assigned this Dec 10, 2020
@robinwhittleton
Copy link
Member

All the four Books themes are working for me, on macOS at least:

ibook themes

Where are you seeing a problem?

@acabal
Copy link
Member Author

acabal commented Dec 10, 2020

On my iPad, they no longer work. Not sure if that's a different app than the desktop version. Fortunately it degrades to still work for titlepage pngs and so on but it doesn't look ideal. I've found other people complaining about the same thing around the web but it looks like there's no solution yet.

@robinwhittleton
Copy link
Member

OK, I’ll test on my phone and see if I can replicate there. What OS version are you running?

@acabal
Copy link
Member Author

acabal commented Dec 10, 2020

iOS 12.4.9

@robinwhittleton
Copy link
Member

I guess it’s Simulator time then :) Let’s get XCode installed.

@robinwhittleton
Copy link
Member

I’ve replicated your issue on my iOS 14 phone, so this is a wider problem.

I’ve checked my macOS copy of Books (macOS 11, Books v3.0) and it still has the correct theme names that we’re using in compatibility.css: __ibooks_internal_theme="BKWhiteStyleTheme", __ibooks_internal_theme="BKSepiaStyleTheme", __ibooks_internal_theme="BKGrayStyleTheme" and __ibooks_internal_theme="BKNightStyleTheme". There’s no way I know of to connect the standard Books debugger to iOS (I’ve got a bountied question open at StackExchange), so I can’t check theme attributes. But one thing I did notice is that Books supports the prefers-color-scheme media query now. That was added in iOS 13 so it won’t fix it for you, but I’ve opened a PR that fixes for later versions. The new media query seems to play nicely with the older theming code at least.

@robinwhittleton
Copy link
Member

Also laughing at this potential solution from Apple’s Books authoring guidelines:

In night mode, the transparent areas of an image will be black. If your image has dark text within a transparent image, that text could be difficult to read in night mode. Instead, we suggest you use a JPEG with a white background.

@acabal
Copy link
Member Author

acabal commented Dec 12, 2020

You know, prefers-color-scheme is a good point. When the compatibility css file was first created, that didn't exist. But now it does, with good browser support and some ereader support. I think it would be worth it to promote that to core.css instead. Then, we would have to update compatibility.css to not invert an image twice on Apple devices that support both that media query and the internal theme selector. What do you think?

@acabal
Copy link
Member Author

acabal commented Dec 12, 2020

Or maybe we don't have to worry about double inverting, I suppose applying invert to the same selector will just override one with the other depending on specificity? And if they're the same then it wouldn't matter anyway.

@robinwhittleton
Copy link
Member

Yep, no worries about double-inversion (unless the one of the filters is on a parent or something). Good idea though, I’ll move the default media query to core.css.

@robinwhittleton
Copy link
Member

I’m still assuming this won’t fix it for your iPad. Perhaps we should remove the theme hack completely? That would mean that older Apple Books would never get the inverted images in dark mode, but that’s presumably better than older MacOS Books working fine and older iOS Books being completely broken.

@acabal
Copy link
Member Author

acabal commented Dec 13, 2020

Yes, I think you're right. If older devices aren't respecting the selector then we should show them pngs with a white background. However we can exclude our pre-generated PNGs like the titlepage and logos, because those have a built-in white border. (I think it may already do that? I don't have it in front of me right now.)

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 a pull request may close this issue.

2 participants