yari
yari copied to clipboard
mdn
Firefox reader mode is broken for some pages
Task List
Issues
I found some pages where opening firefox's reader mode would display incomplete content:
Doc for the WWW-Authenticate HTTP header
In this case, only the browser compatibility table seems to have made it in. Other pages for http headers seem to behave the same way, although some just don't have a reader mode, possibly due to very short content.
The obvious culprit seems to be the new theme, but I can't say for sure. It worked fine 2 months ago
Other pages seem to handle reader-mode better than other, for example the doc for Array.find() seems to work fine, only the compatibility table seems broken (and, again, it seemed better a month ago). I've been through most of the Array pages and they all seem to behave the same way.
I just did a quick check just now, and it looks like it's almost fixed! The compatibility table is still missing in reader mode, but the rest looks exactly like it used to!
That is great news @quentinvernot! Thank you for verifying. I believe the problem with the compatibility table is probably related to how it is loaded. @mdn/core-dev not sure how hard this would be, and perhaps the future plans for Yari will already address this, but I thought I would at least put it on your radar.
Firefox Reader mode seems to omit tables, because it isn't trivial how to render them in a readable way:
I will discuss this with the rest of the MDN engineering team, but I don't think there is much that we can do.
The Firefox team pointed me to https://github.com/mozilla/readability/blob/master/Readability.js, which is the underlying implementation that decides what elements to show.
It could be that the scroll in the table-scroll class is causing the element to be hidden:
https://github.com/mozilla/readability/blob/1d2cb030b32e753cc4b7c4ce8b64c3ce4dc1b2ff/Readability.js#L128
FWIW Reverting https://github.com/mdn/yari/pull/5560 indeed makes the table re-appear in the Reader View, whereas just changing or removing the table-scroll class does not seem to do the trick. 🤔
Hey! Thanks for the fix, I can confirm that tables are working now! Unfortunately, the compatibility table at the end of the page is still broken.
Looking into it, it looks like this table is generated from a request to <page url>/bcd.json, which was already the case with the old theme so that's probably not the issue. The table is wrapped in a double <div> however, to have horizontal scrolling in narrow viewports, and seeing how it affected the other tables, I'd bet this is the cause of the issue here too. The wrapping is probably here for a good reason though: it's a wide and dense table, it even has responsive styles for various viewport widths, so I'm not sure how it can be fixed, I don't know this codebase enough to know where the look.
I got screenshots though:
Would you like me to open a separate issue for this?
Glad to hear it's working now! 🙂
Would you like me to open a separate issue for this?
I think this should be covered by https://github.com/mdn/yari/issues/3653, i.e. resolving that issue should also resolve this. (And now this issue is referenced there.)
As long as we don't server-side render BCD tables, they won't appear in the Reader Mode. 😉
Alright, I'll keep an eye on https://github.com/mdn/yari/issues/3653. If that still doesn't fix the issue, I'll re-open something then.
Thank you!