Closed
Bug 1259763
Opened 9 years ago
Closed 8 years ago
Reader mode omits opening paragraph on CNN articles
Categories
(Toolkit :: Reader Mode, defect, P3)
Toolkit
Reader Mode
Tracking
()
RESOLVED
FIXED
People
(Reporter: abr, Assigned: evanxd)
References
(Blocks 3 open bugs)
Details
(Whiteboard: [reader-mode-readability-algorithm])
Articles on CNN frequently have an opening paragraph that is styled differently than others. Reader mode does not include this lead paragraph in its view.
See, for example, http://money.cnn.com/2016/02/01/news/economy/poverty-inequality-united-states/index.html
Comment 1•9 years ago
|
||
Thanks for the report. This sounds like an issue with the Readability library, so I filed an issue here: https://github.com/mozilla/readability/issues/281
Updated•8 years ago
|
Priority: -- → P3
Updated•8 years ago
|
Whiteboard: [reader-mode-readability-algorithm]
Assignee | ||
Comment 2•8 years ago
|
||
Cannot reproduce anymore. The webpage[1] seems already changed. There is no opening paragraph there and the reader mode result is good.
[1]: http://money.cnn.com/2016/02/01/news/economy/poverty-inequality-united-states/index.html
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Comment 3•8 years ago
|
||
(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #2)
> Cannot reproduce anymore. The webpage[1] seems already changed. There is no
> opening paragraph there
I see:
The U.S. has long been heralded as a land of opportunity -- a place where anyone can succeed regardless of the economic class they were born into.
on the page in a different font, and that paragraph does not make it into the reader mode result. Are you seeing something else?
> and the reader mode result is good.
>
> [1]:
> http://money.cnn.com/2016/02/01/news/economy/poverty-inequality-united-
> states/index.html
Flags: needinfo?(evan)
Comment 4•8 years ago
|
||
Gijs I see the same. I do know of cases where large sites to serve different markup to different regions.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Updated•8 years ago
|
Status: REOPENED → NEW
Assignee | ||
Comment 5•8 years ago
|
||
I can reproduce the issue mentioned on Comment 3. Somehow the `<h2>The U.S. has long been heralded as a land of opportunity -- a place where anyone can succeed regardless of the economic class they were born into.</h2>` node is just removed by some kind of reason.
Good thing is the algorithm chooses correct `topCandidate` (`<div id="storytext">`).
Continue investigate the issue...
Flags: needinfo?(evan)
Assignee | ||
Comment 6•8 years ago
|
||
Added tests[1] to investigate the issue.
[1]: https://github.com/mozilla/readability/pull/347/commits/077bca8721975efa607839f8c2756d2eee323f29
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → evan
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•8 years ago
|
||
Sent a PR[1] with the solution. Let's discuss it there.
[1]: https://github.com/mozilla/readability/pull/347/commits/a0f94b1869b5188dfad66d1d5cc8b6270c5bc4f2
Assignee | ||
Comment 8•8 years ago
|
||
Updated the patch to use a new solution to fix the issue[1].
[1]: https://github.com/mozilla/readability/pull/347/commits/64e97fead34ed567025109c2b6df0ae2d8a40db4
Assignee | ||
Comment 9•8 years ago
|
||
Fixed all test failures[1].
[1]: https://github.com/mozilla/readability/pull/347/commits/73a020d56675ba649a272d95e025404a9d7936f5
Assignee | ||
Comment 10•8 years ago
|
||
Updated patch for review comments: https://github.com/mozilla/readability/pull/347/commits/e6ae86bd9c4dc9d14f87d16f99f4a57b319ce4bb
Assignee | ||
Comment 11•8 years ago
|
||
Assignee | ||
Comment 12•8 years ago
|
||
We'll land it in m-c in the MozReview patch[1].
[1]: https://reviewboard.mozilla.org/r/109976/diff/2#index_header
Assignee | ||
Comment 13•8 years ago
|
||
Landed in m-c: https://hg.mozilla.org/mozilla-central/rev/d1bef3268b21
Assignee | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•