Fun fact: dyslexic font in night mode is mostly renderered black/dark (invisible), other fonts okay.

danielweck commented 5 years ago

OpenType issue? Chromium-specific? (reproducible with r2-testapp-js and readium-desktop, I haven't tried elsewhere) Book/CSS-specific? (Screenshots below with "Accessible EPUB3")

...to be determined.

screenshot 2019-01-03 at 17 16 19

screenshot 2019-01-03 at 17 17 39

JayPanoz commented 5 years ago

Hmmm hmmm hmmm thanks for reporting that. After a quick check, there’s nothing related to colours in the a11y-font submodule so this is kinda weird, especially as headings and code are inverted properly.

Kinda reminds me of an issue I encountered with a KePub file some months ago though, that I didn’t consequently flagged as an issue because well, there’s extra markup + JS specific to Kobo apps/devices in those files. Still this one is weird because it only happens with the dys font.

If possible, could you also provide debug tools’ applied styles panel, even if in the form of a screenshot?* Not the computed styles, just the applied one in order to get a sense of the cascade and how stylesheets interact there.

* I should probably add that in the issue template as it can help quite tremendously and often save a lot of time trying to reproduce it.

danielweck commented 5 years ago

Bug confirmed using the "font testing tool" with the new "night mode" switch: https://readium.org/readium-css/docs/utils/Typeface-tester.html (screenshots below)

danielweck commented 5 years ago

Normal mode (green oval annotation added by me):

readiumcss_accessibledfa_nightmode_1

danielweck commented 5 years ago

Night mode (green oval annotation added by me):

readiumcss_accessibledfa_nightmode_2

JayPanoz commented 5 years ago

Thanks for testing that before I could do.

So that’s kinda insane, I’ve never seen a bug like this one.

I’ll file the issue in the Orange repo ASAP. In the meantime I guess using OpenDyslexic as a replacement would be the best option.

danielweck commented 5 years ago

Here is a reduced test case (in the second paragraph, the text color should be red, it remains black):

<html>
<head>
<style>
@font-face {
  font-family: AccessibleDfA;
  font-style: normal;
  font-weight: normal;
  src: local("AccessibleDfA"),
    url("https://raw.githack.com/Orange-OpenSource/font-accessible-dfa/master/All Fonts/AccessibleDfA.otf") format("opentype");
}
</style>
</head>
<body style="font-family: AccessibleDfA;">
<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum
</p>
<hr>
<p style="color: red;">
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum
</p>
</body>
</html>

danielweck commented 5 years ago

Copy/paste data: URL (same HTML content as above, base64-encoded):

data:text/html;base64,PGh0bWw+CjxoZWFkPgo8c3R5bGU+CkBmb250LWZhY2UgewogIGZvbnQtZmFtaWx5OiBBY2Nlc3NpYmxlRGZBOwogIGZvbnQtc3R5bGU6IG5vcm1hbDsKICBmb250LXdlaWdodDogbm9ybWFsOwogIHNyYzogbG9jYWwoIkFjY2Vzc2libGVEZkEiKSwKICAgIHVybCgiaHR0cHM6Ly9yYXcuZ2l0aGFjay5jb20vT3JhbmdlLU9wZW5Tb3VyY2UvZm9udC1hY2Nlc3NpYmxlLWRmYS9tYXN0ZXIvQWxsIEZvbnRzL0FjY2Vzc2libGVEZkEub3RmIikgZm9ybWF0KCJvcGVudHlwZSIpOwp9Cjwvc3R5bGU+CjwvaGVhZD4KPGJvZHkgc3R5bGU9ImZvbnQtZmFtaWx5OiBBY2Nlc3NpYmxlRGZBOyI+CjxwPgpMb3JlbSBpcHN1bSBkb2xvciBzaXQgYW1ldCwgY29uc2VjdGV0dXIgYWRpcGlzY2luZyBlbGl0LCBzZWQgZG8gZWl1c21vZCB0ZW1wb3IgaW5jaWRpZHVudCB1dCBsYWJvcmUgZXQgZG9sb3JlIG1hZ25hIGFsaXF1YS4gVXQgZW5pbSBhZCBtaW5pbSB2ZW5pYW0sIHF1aXMgbm9zdHJ1ZCBleGVyY2l0YXRpb24gdWxsYW1jbyBsYWJvcmlzIG5pc2kgdXQgYWxpcXVpcCBleCBlYSBjb21tb2RvIGNvbnNlcXVhdC4gRHVpcyBhdXRlIGlydXJlIGRvbG9yIGluIHJlcHJlaGVuZGVyaXQgaW4gdm9sdXB0YXRlIHZlbGl0IGVzc2UgY2lsbHVtIGRvbG9yZSBldSBmdWdpYXQgbnVsbGEgcGFyaWF0dXIuIEV4Y2VwdGV1ciBzaW50IG9jY2FlY2F0IGN1cGlkYXRhdCBub24gcHJvaWRlbnQsIHN1bnQgaW4gY3VscGEgcXVpIG9mZmljaWEgZGVzZXJ1bnQgbW9sbGl0IGFuaW0gaWQgZXN0IGxhYm9ydW0KPC9wPgo8aHI+CjxwIHN0eWxlPSJjb2xvcjogcmVkOyI+CkxvcmVtIGlwc3VtIGRvbG9yIHNpdCBhbWV0LCBjb25zZWN0ZXR1ciBhZGlwaXNjaW5nIGVsaXQsIHNlZCBkbyBlaXVzbW9kIHRlbXBvciBpbmNpZGlkdW50IHV0IGxhYm9yZSBldCBkb2xvcmUgbWFnbmEgYWxpcXVhLiBVdCBlbmltIGFkIG1pbmltIHZlbmlhbSwgcXVpcyBub3N0cnVkIGV4ZXJjaXRhdGlvbiB1bGxhbWNvIGxhYm9yaXMgbmlzaSB1dCBhbGlxdWlwIGV4IGVhIGNvbW1vZG8gY29uc2VxdWF0LiBEdWlzIGF1dGUgaXJ1cmUgZG9sb3IgaW4gcmVwcmVoZW5kZXJpdCBpbiB2b2x1cHRhdGUgdmVsaXQgZXNzZSBjaWxsdW0gZG9sb3JlIGV1IGZ1Z2lhdCBudWxsYSBwYXJpYXR1ci4gRXhjZXB0ZXVyIHNpbnQgb2NjYWVjYXQgY3VwaWRhdGF0IG5vbiBwcm9pZGVudCwgc3VudCBpbiBjdWxwYSBxdWkgb2ZmaWNpYSBkZXNlcnVudCBtb2xsaXQgYW5pbSBpZCBlc3QgbGFib3J1bQo8L3A+CjwvYm9keT4KPC9odG1sPg==

danielweck commented 5 years ago

I logged an issue here: https://github.com/Orange-OpenSource/font-accessible-dfa/issues/2

JayPanoz commented 5 years ago

Thanks for letting me know, I was just about to log it too. 👍

danielweck commented 5 years ago

Web browsers (MacOS): 1) Safari (works as intended, second paragraph is red) 2) Firefox (works as intended, second paragraph is red) 3) Chrome (does not work, second paragraph is black)

JayPanoz commented 5 years ago

Hmmm interesting that reminds me of the inconsistencies when testing Literata (#58)

@danielweck by any chance, are you using MacOS Mojave? Correlation ≠ causation but well, maybe I should retest some fonts just in case.

danielweck commented 5 years ago

Yes, MacOS Mojave 10.14.2.

Have you tried the reduced test case? (HTML snippet / data: URL) Are you able to reproduce the bug in all web browsers but WebKit/Safari?

danielweck commented 5 years ago

By the way, the test above is with the OTF deliverable, but I am able to reproduce with the TTF and WOFF variants too.

https://github.com/Orange-OpenSource/font-accessible-dfa/tree/master/All%20Fonts

JayPanoz commented 5 years ago

Yes, that was the reason why I was asking, sorry if that was unclear.

JayPanoz commented 5 years ago

So after super quick testing I can report Safari won’t render local fonts that were installed by the user (Font Book > user fonts) but other browsers will without any issue.

So that makes the Literata bug in #58 a system bug, at least to me.

I’ll try to dig a little bit deeper as soon as possible, esp. color and non-local fonts.

danielweck commented 5 years ago

Okay, I am being an idiot here (sorry it's late ...) I forgot to remove src: local("AccessibleDfA") from the @font-face declaration ... trying again with the remote URLs only.

danielweck commented 5 years ago

Yeah, only Chrome fails, Safari and Firefox work with OTF, TTF and WOFF. UPDATE: nope, now everything works fine! https://accessible-dfa-color.surge.sh/ReadiumCSS_AccessibleDFA_NightMode.html (still trying to find a minimal repro test case)

JayPanoz commented 5 years ago

As a clarification, just to be sure. Here is how it fails for me in Safari when I say local fonts installed by the user so not even src: local("font") in that case:

capture d ecran 2019-01-23 a 23 13 42

Chrome + Firefox are OK.

capture d ecran 2019-01-23 a 23 16 05

There’s definitely something wrong going on with local fonts (user-installed so double-click on fonts and then install in Font Book.app) on my machine.

danielweck commented 5 years ago

Damn, after clearing the browser cache, removing the local font, dealing with CORS issues, and setting up an HTTPS test URL, ... I am not able to reproduce the bug. I have closed the other issue until we find out exactly how to reproduce the "bug" using a minimal reproducible test case. Sorry for the noise. https://accessible-dfa-color.surge.sh/ReadiumCSS_AccessibleDFA_NightMode.html

danielweck commented 5 years ago

Note that the originally-reported bug in the Readium2 "desktop" app (Electron/Chromium) still exists. I am just trying to create a more generic test.

danielweck commented 5 years ago

Ah, so in r2-testapp-js (and readium-desktop) the font is loaded from this HTTPS URL: https://127.0.0.1:8000/readium-css/fonts/AccessibleDfA.otf (localhost NodeJS Express server, i.e. r2-streamer-js with a static route to the ReadiumCSS folder)

So, I ran a test by loading this very same font into the iframe / webview host (i.e. the main index.html of the Electron renderer process / Chromium window), by injecting this CSS snippet using the web inspector at runtime:

@font-face {
  font-family: AccessibleDfA;
  font-style: normal;
  font-weight: normal;
  src: local("AccessibleDfA"),
    url("https://127.0.0.1:8000/readium-css/fonts/AccessibleDfA.otf") format("opentype");
}

(effectively the same @font-face declaration used when ReadiumCSS is injected into EPUB HTML Content Documents)

...then I added this attribute using the web inspector to some text in the app's user interface: style="font-family: AccessibleDfA !important; color: red; !important" ...and the text is red as expected!

If I disable ReadiumCSS and manually inject the exact same above CSS snippet inside the DOM inside the hosted / embedded webview, then the text color remains black! Weird bug.

danielweck commented 5 years ago

Okay, I found a workaround. font-weight: bold fixes the problem. Don't ask me why. Good night :)

JayPanoz commented 5 years ago

Alright, I’ve done some extra research during breakfast (sorry couldn’t post this morning as I was involved in a project defence jury @ the University).

@danielweck My best guess is that when you’ve been initially testing and Safari was OK, it didn’t use the local version of the font (user-installed) but the online fallback because – surprise! – Safari doesn’t render user-installed local fonts anymore due to fingerprinting/privacy concerns.

See this reddit thread and this SO question. There’s a lot more of such questions/thread in the wild but few with a technical background so listing the 2 most relevant.

I do believe that we’d better trim the issue down to Chrome and Firefox. My plan right now is the following:

check AccessibleDfa’s versions (local and online) so I’ll also give a deeper look at the fonts in FontForge to check metadata/hypothetical issues;
if both are the same version, I’m planning to check various fonts I have installed on my machine, in order to see if the issue is reproducible with other fonts.

Then I’ll improvise.

At best I can init that tonight.

font-weight: bold fixes the problem

Wouldn’t that “create” an extrapolated weight of the (base) regular font as there isn’t any bold font available?

My worry is that clearly Apple has been changing something in Safari and they may well have modified how MacOS manages local fonts to enable that. If that’s the case, we should be aware, because that would basically prevent us from using local as src, just to be on the safe side of the fence.

JayPanoz commented 5 years ago

@danielweck So I checked versions and we were lagging quite a little bit – we were @ 1.0001 while latest is @ 1.5.

I can confirm that after the latest version is installed locally, it’s working as expected in Chrome and Firefox (i.e. black/red).

That brings us back to one of the issues mentioned in #58 though: updating fonts regularly if we decide to embed them in RCSS itself.

danielweck commented 5 years ago

Status update: I downloaded the latest version of AccessibleDfA.otf from https://github.com/Orange-OpenSource/font-accessible-dfa/tree/master/All%20Fonts ...and I tested r2-navigator-js / r2-testapp-js with it: the bug still exists, i.e. the foreground colour does not change inside a webview of Electron 1.8.8 (Chromium 59.0.3071.115 https://github.com/electron/libchromiumcontent/blob/electron-1-8-x/VERSION ), unless font-weight: bold is applied (there might be other workarounds, but this one works).

I haven't tried more recent version of Electron/Chromium yet.

I haven't had time to retrofit this simple test inside an Electron app yet (iframe, not webview ... but I guess we should test both): https://accessible-dfa-color.surge.sh/ReadiumCSS_AccessibleDFA_NightMode.html

danielweck commented 1 year ago

fixed by https://github.com/readium/readium-css/pull/123 ?

mickael-menu commented 1 year ago

That does look very similar. Maybe they fixed the font a first time but forgot some glyphs like the comma.

JayPanoz commented 2 months ago

I’m assuming #123 fixed this so closing the issue, unless it pops up again.

readium / readium-css

Fun fact: dyslexic font in night mode is mostly renderered black/dark (invisible), other fonts okay. #56