Closed Bug 1153384 Opened 8 years ago Closed 8 years ago
Reader Mode no longer triggers on many pages, including ones that worked earlier
39 bytes, text/x-review-board-request
Here are some Austrian news sites (and articles) where Reader Mode doesn't work. On many of this pages it worked a few weeks ago: http://derstandard.at/2000014110146/Entwurf-zu-Rauchverbot-steht (Failed to load article from page) http://orf.at/stories/2272872/ (no Reader icon appears) http://kurier.at/politik/weltchronik/usa-selbstmordanschlag-auf-militaerbasis-in-kansas-vereitelt/124.364.599 (also no Reader icon) Some articles do work, for example: http://derstandard.at/2000014152981/Mailand-zittert-der-Expo-entgegen Don't know what's different with this one. Looks pretty arbitrary to me which articles work with Reader Mode and which don't.
(In reply to Markus Popp from comment #0) > Here are some Austrian news sites (and articles) where Reader Mode doesn't > work. On many of this pages it worked a few weeks ago: > > http://derstandard.at/2000014110146/Entwurf-zu-Rauchverbot-steht (Failed to > load article from page) This now works in beta (and presumably Nightly) > http://orf.at/stories/2272872/ (no Reader icon appears) This is still broken. > http://kurier.at/politik/weltchronik/usa-selbstmordanschlag-auf- > militaerbasis-in-kansas-vereitelt/124.364.599 (also no Reader icon) This seems to now work as well? Can you confirm this, or are you still seeing the same issues?
That's correct, I'm getting the same results in both Beta 7 and Nightly (of April 24th).
/r/7681 - Bug 1153384 - improve isProbablyReaderable detection, r?margaret Pull down this commit: hg pull -r 9316c968b30d082bd268b0d043244bc9a940c481 https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8598043 [details] MozReview Request: bz://1153384/Gijs See also: https://github.com/mozilla/readability/pull/188
Attachment #8598043 - Flags: review?(margaret.leibovic)
Comment on attachment 8598043 [details] MozReview Request: bz://1153384/Gijs https://reviewboard.mozilla.org/r/7679/#review6459 ::: toolkit/components/reader/ReaderMode.jsm:121 (Diff revision 1) > + return new Readability(uri, doc).isProbablyReaderable(this.isNodeVisible.bind(this, utils)); Maybe add a comment here explaining why we need to pass in this helper function? I also wonder if passing a helper function like this to Readability.parse() could also be a better way of removing hidden elements like I was trying to do in this pull request... https://github.com/mozilla/readability/pull/186 But then again, having `isProbablyReaderable` depend on some gecko functionality feels more acceptable than having `parse` depend on it.
Attachment #8598043 - Flags: review?(margaret.leibovic) → review+
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8598043 [details] MozReview Request: bz://1153384/Gijs Approval Request Comment [Feature/regressing bug #]: reader mode [User impact if declined]: reader mode not available for pages where it should be [Describe test coverage new/current, TreeHerder]: has tests in github repo [Risks and why]: medium-low, low because only affects github repo, medium-ish because it changes when we show the reader mode button [String/UUID change made/needed]: nope.
Whiteboard: [do not verify before bug 1158228 has landed]
Comment on attachment 8598043 [details] MozReview Request: bz://1153384/Gijs Approving for uplift to aurora and to beta (from discussion with Sylvestre). We want this to be in the 38.0.5 but not in 38.0, since Reader Mode is disabled for 38.0 on Desktop.
You need to log in before you can comment on or make changes to this bug.