"Obsolete" boxes don't render in Firefox reader mode

NEW
Unassigned

Status

developer.mozilla.org
General
--
minor
3 years ago
3 years ago

People

(Reporter: sheppy, Unassigned)

Tracking

({in-triage})

Details

(Whiteboard: [specification][type:bug])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
What did you do?
================
1. I visited https://developer.mozilla.org/en-US/docs/Web/HTML/Element/marquee
2. Clicked the reader mode button in the URL bar

What happened?
==============
The page redraws in reader mode, but the "Obsolete" banner at the top of the page does not draw in any fashion -- not even just as a paragraph of text.

What should have happened?
==========================
The obsolete notice should appear in some form at the top of the page, however is practicable with the reader mode in effect.

Odds are this affects other banners as well, such as "deprecated" and so forth.

Is there anything else we should know?
======================================
(Reporter)

Comment 1

3 years ago
Created attachment 8656133 [details]
Screen shot of the warning on the <marquee> page, normally
(Reporter)

Comment 2

3 years ago
Created attachment 8656136 [details]
Screen shot of the <marquee> page in reader mode, sans warning box

I've attached screen shots of the <marquee> page, one when browsing normally (note the big orange "Obsolete" box) and one when using reader mode (note the lack of any indication at all that <marquee> is bad).
Severity: normal → minor
Keywords: in-triage
It's not clear now whether this is a bug in reader mode or a bug on MDN.

Comment 4

3 years ago
You could use readability from the commandline with node to look at exactly what is being stripped. We run it in a worker, and I don't know how far along worker JS debugging is - if it works these days, you might be able to use the browser debugger to poke at it directly.

At a guess, I would imagine that the classes on the div are causing it to get dropped. Here's the heuristics that get used: https://github.com/gijsk/readability/blob/master/Readability.js#L107-L123 . I imagine the naming speaks more or less for itself.

I'm not really sure what to suggest without having a more exact idea of what is going wrong, and I don't have time to investigate this today because I just got back from PTO and have a mountain of things to get through. Please needinfo me if/when you have more info or questions.

As an afterthought, it'd also be good to have a better idea of whether this affects all banners/infoboxes, or specifically the "obsolete" ones, or...?

If it's easy to try changing something locally, a quick experiment you could try would be killing off the extra level of nesting (it's a div with only a classless p in it, so I dunno offhand why the p isn't the thing with classes and lives at the toplevel...).

Comment 5

3 years ago
(In reply to Gijs Kruitbosch from comment #4)
> https://github.com/gijsk/readability/blob/master/Readability.js#L107-L123 .

Bleh, awesomebar. https://github.com/mozilla/readability/blob/master/Readability.js#L107-L123 would be the official repo, though my clone is up-to-date.
(Reporter)

Comment 6

3 years ago
I was thinking that the existence of "header" in the class list for an element was triggering blocks from being removed, but I've found exceptions.

On https://developer.mozilla.org/en-US/docs/Web/API/LocalFileSystemSync, there's a non-standard box (as <div>) with the classes overheadIndicator, nonStandard, and nonStandardHeader. The contents of the box appear in reader mode.

On https://developer.mozilla.org/en-US/docs/Web/API/AudioProcessingEvent, there's a "deprecated" box, also a <div>, with the classes overheadIndicator, deprecated, and deprecatedHeader. This box does *not* appear in reader mode.

I don't understand what's happening here, but hopefully having this information will help someone figure it out...

Updated

3 years ago
Flags: needinfo?(gijskruitbosch+bugs)

Comment 7

3 years ago
The solution turns out to be quite simple: |header| indeed triggers removal here, and "nonStandard" triggers keeping the element because it contains "and", which is in the "okMaybeItsACandidate" regex.

Modifying one of the classes and/or these templates to consistently contain "article" or "body" or "column" or "main" or "shadow" would make them work.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 8

3 years ago
(In reply to :Gijs Kruitbosch from comment #7)
> The solution turns out to be quite simple: |header| indeed triggers removal
> here, and "nonStandard" triggers keeping the element because it contains
> "and", which is in the "okMaybeItsACandidate" regex.

So, to be clear, this is why the header in the first article whose class list looks like: "overheadIndicator deprecated deprecatedHeader" disappears, and the other, whose class list looks like "overheadIndicator nonStandard nonStandardHeader" does not.
You need to log in before you can comment on or make changes to this bug.