1472749 - reader view of NYTimes.com article elides much of the text (sync from github 7d03bec52d0a0c4b22d044e06af84abb15a9f02b )

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect, P1)

Description

[Raymond Peck]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180607193903

Steps to reproduce:

Went to:

https://www.nytimes.com/2018/06/25/upshot/i-learned-i-have-sleep-apnea-its-more-serious-than-many-people-realize.html

Clicked on the "reader mode" icon.


Actual results:

"Reader mode" doesn't show the first few paragraphs of the text.

Comment 1

[Gingerbread Man]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
20180621125625

Maybe I'm missing something, but I don't see any of the first paragraphs absent in reader mode.
This problem was previously fixed in bug 1300697.

Comment 2

[:Gijs (he/him)]

I can reproduce the problem (tested on 62 beta on macOS), though it goes away if I refresh the resulting reader view. Refreshing causes us to load the original HTML for the article off the network (without any styling or running any more script or something like that), instead of using the page that's been rendered in the browser for the original load of the article (which *will* have had the styling/script/whatever). I therefore expect there's something that script on the nytimes' website is doing that's altering the markup in a way that's making it harder for reader mode to extract the important parts of the content...

Tentatively marking as P1 because we should ideally work properly on the nytimes website...

Raymond, do you know if this affects all articles you've tried to use reader mode for on nytimes.com, or just some of them?

Comment 3

[:Gijs (he/him)]

I put up a fix on https://github.com/mozilla/readability/pull/469 .

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9682b4ae989
fix reader mode on nytimes website (sync from github 7d03bec52d0a0c4b22d044e06af84abb15a9f02b), r=jaws

Comment 5

[:Gijs (he/him)]

Attachment: Patch for beta

Approval Request Comment
[Feature/Bug causing the regression]: n/a, nytimes change
[User impact if declined]: can't successfully use reader mode with nytimes articles with "too many" ads in the page
[Is this code covered by automated tests?]: reader mode has general tests, yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: see comment #0
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: one-line change, we have a lot of beta runway, this will only affect reader mode
[String changes made/needed]: no

(This patch should apply cleanly on beta; there have been on changes to reader mode since 62 branched.)

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d9682b4ae989

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8990989 [details]
Patch for beta

Fix in reader mode, looks low-risk, let's take this for beta 8.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dc27393652a6

Comment 9

[Damira Akh]

Tested on Both Beta and Latest Nightly. 
The issue is not appearing anymore.


Build ID 	20180605171542
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0


Build ID 	20180713100116
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0