readium/readium-js

bug: author-defined text colors no longer rendering in all settings

thedirk opened this issue · 13 comments

thedirk commented

Bug in new release (0.29 Chrome App):

Expected Behavior

Previous releases in Readium performed as expected with regard to colored text.

In Settings > Style > Text & Background Color, any theme selected retained hard-coded color-coding (text colors defined by HTML span tags)... While default text colors (e.g., plain black text) varied according to theme.

In Settings > Layout > Scroll mode, hard-coded color text displayed as colored text throughout all three options.

Observed behavior

The current release of Readium does not perform as expected with regard to colored text.

In Settings > Style > Text & Background Color, only "Default Author Styles" properly displays text colors. All other themes for all text to "Theme Default" color.

In Settings > Layout > Scroll mode, hard-coded color text displays properly only on the "Auto" setting. Both "Document" and "Continuous scroll" settings force all text to default.

Steps to reproduce

  1. First, create an EPUB with color text. (Mine is defined in the .xhtml text file, I haven't tested for CSS defined color text). Or, use my test file (below).
    For example:

    Here is some text in color

  2. Then, import the EPUB into the Google Chrome Readium App.

  3. Open it.

  4. Change the settings (particularly Scroll Mode and Theme)—the color used to be stable throughout these options (as expected), and is no longer.

Test file(s)

https://drive.google.com/open?id=1wgQHQRZm3crGD9zYqgdYX7oEgf_KZKcY

Product

  • Readium Chrome extension
    • latest official version available from the Chrome Web Store
    • Version 63.0.3239.84 (Official Build) (64-bit)
    • Windows 10 (64 bit)
rkwright commented

@danielweck @jccr
Confirmed that this is bug. Also verified that this is a REGRESSION. I only tested against 0.23, but it works fine there. In the latest (0.31-alpha) the theme overrides the user's specified colors. See the sample file here.

danielweck commented

Actually, the implementation of "themes" conforms to the expected behaviour ... if we accept that colour schemes consist in both a background and a foreground colour, consistently enforced into every part of the document.

I am personally open to revisit the definition of the feature, and to investigate ways to implement it correctly ... but if I remember correctly there were low-contrast issues because of background and foreground colours being selected from different sources, i.e. in cases where arbitrary / unpredictable author-specified colours where inadequately mixed with the forced background colour defined by the user-chosen theme. This is why we ended-up fixing the implementation by enforcing both background and foreground in the entire document, thereby overriding author styles entirely.

In a nutshell, if we allow authored colours to take precedence over the user-chosen theme, then we will re-open the issue of inappropriate colour combinations / low contrast / broken accessibility.

rkwright commented

Hm I am reluctant to do so much work on R1 at this point.
Given that R1 now uses readium.css (in 0.31-alpha, correct?) is the behaviour of R2 the same? I'll test when I get upgraded to the current rev.

danielweck commented

I'm not sure whether "ReadiumCSS" background+foreground colour scheme takes precedence over entire documents, thereby discarding authored styles. @JayPanoz?

JayPanoz commented

I'm not sure whether "ReadiumCSS" background+foreground colour scheme takes precedence over entire documents, thereby discarding authored styles.

I can confirm that they are at the moment as we’re using !important, cf. day mode module.

That also happens to be an issue reported by @jccr on the Readium CSS issue tracker a while ago. The rationale for this (temporary?) choice was indeed about a11y contrast + several ebooks forcing their own “theme” (e.g. sepia), which broke the readers’ expectations of the reading modes (no day mode available in such a case).

As stated in the Readium CSS issue, I’m open to discuss our options since it puts a burden on authors.

JayPanoz commented

And, further details: for sepia and night modes, we pretty much implemented them with interoperability in mind i.e. sepia mode tries to not override authors’ colors but night mode will override any color as we can’t guarantee contrast/vibrance/etc. will be OK – there are way too many files whose text colors fail in iBooks’ or Kobo’s gray mode for instance, and it creates awful a11y issues.

danielweck commented

Note that iBooks' "sepia" theme does not apply !important to foreground colour (thanks @jccr for reporting this) , unlike its two other colour schemes. Presumably, this is because the contrast ratio of black-on-white is quite similar to brown-on-beige, which would justify ; from a UX perspective ; preserving the author's styles/foreground colours (?).
So, Ric's test EPUB renders red, blue and orange spans in iBooks with "sepia", but not the other themes.

Perhaps in Readium we should apply the same logic to "Sands Of Dune" (sepia) and "Vancouver Mist"? Test here: https://readium.firebaseapp.com

danielweck commented

As you can see from the above links, the setStyles() function in helpers.js injects !important only when the styles are applied to the document object (which is the default behaviour for setting "book styles"). In order to differentiate "Sands Of Dune" (sepia) and "Vancouver Mist" (silver/gray), we would have to make relatively significant changes to the existing logic, so this is yet another reason to carefully consider the UX pros/cons before implementing the technical solution.

Also, Readium2 / ReadiumCSS is where these kinds of issues are enthusiastically discussed, whereas Readium1 is entering maintenance mode, if I understand correctly. At any rate, @JayPanoz let us know if you have thoughts about the UX (we have a pretty good idea on how to technically address this in Readium1, especially because the solution would not be pure-CSS ... unlike ReadiumCSS at the moment).

PS: note that iBooks utilizes a top-level attribute on the root HTML element to define the colour scheme, probably so that they can leverage CSS selector specificity (in addition to !important), in order to enforce certain styles in the document cascade.

JayPanoz commented

note that iBooks utilizes a top-level attribute on the root HTML element to define the colour scheme, probably so that they can leverage CSS selector specificity (in addition to !important), in order to enforce certain styles in the document cascade.

We pretty much have the same strategy in ReadiumCSS Alpha:

With ReadiumCSS implementing features that also exist in other apps (blending images in sepia mode, inverting gaiji in night mode, etc.) in CSS, while other apps may be implementing them in another fashion.

UX-wise, things can work like that in sepia, but for apps offering lightblue or mint backgrounds, not so much (e.g. blue links on a light blue background, green/red headings on a mint background, etc.).

They typically don’t override colors and treat them as they do in sepia mode, but in my humble experience, it made those themes useless with some publications – and such pastel backgrounds are often used for dyslexia for instance, so it’s also a11y-related.

Note that we’ll also have themes in ReadiumCSS Beta, so I’ll open a “discussion” issue there as well. In anticipation of themes, I fantasized sepia and night modes being redesigned as light and dark modes themes can actually reuse, but that’s easier said than done, given all light themes can not be created equal, as illustrated in the two previous paragraphs above.