search

readium / readium-js-viewer

👁 ReadiumJS viewer: default web app for Readium.js library
BSD 3-Clause "New" or "Revised" License
550 stars 186 forks source link

Media overlay highlights don't render with themes #678

Open jccr opened 6 years ago

jccr commented 6 years ago

This is about if a user has a theme option applied, and media overlays are visually syncing with highlighting.

This issue is a Bug

Related issue(s) and/or pull request(s)

May have some similarities with #664

Expected Behaviour

Display highlights in this scenario, like it does with no theme applied.

Observed behaviour

When a theme is used, no highlights are showing.

Steps to reproduce

  1. Open the Moby Dick Sample EPUB with Media Overlays.
  2. Go to chapter 1 and begin playback
  3. Check that highlights are showing up with the playback
  4. Stop the media overlay player
  5. Open the settings in the reader and set a theme
  6. Go back to Chapter 1 and begin playback
  7. Check that highlights are showing up

Test file(s)

https://readium.firebaseapp.com/?epub=https%3A%2F%2Fcdn.rawgit.com%2FIDPF%2Fepub3-samples%2Fmaster%2F30%2Fmoby-dick-mo&epubs=epub_content%2Fidpf_samples.opds&goto=epubcfi(/6/14!/4/2/2%5Bc01h01%5D/1:0)

Product

danielweck commented 6 years ago

Actually that is partially true :) EPUB3 Media Overlays (MO) highlights do work when a Reading System (RS) theme is activated, and when a custom Readium MO highlight style is chosen by the user. MO highlights only fail when the author CSS for the "MO active" class does not specify ! important (in which case the RS theme CSS takes precedence). So, to fix this, we can either: 1) bump-up the cascade priority for the author CSS (technically difficult to implement across all Readium target platforms, as Readium would have to dynamically modify publication styles programmatically via the CSS OM rules API, or ahead-of-time by pre-processing static CSS files / declarations). 2) downgrade the cascade priority for the RS themes (easy to implement by removing ! important, but will lead to many situations where the authored styling will not be overridden by the user-chosen color schemes) Thoughts?

danielweck commented 6 years ago

Actually there is a third option: completely revisit how theme styles and MO highlights are applied, and craft the CSS cascade / rules specificity in such a way that authored styling "wins" where necessary (i.e. smarter use of top-level CSS selectors, style and class attributes, and ! important).

rkwright commented 6 years ago

This is a nice-to-have and might "come for free" if/when we support readium.css in R1, but that is not a trivial amount of work and testing. So not a 0.29 item for sure.