Closed Bug 1444082 Opened 4 years ago Closed 4 years ago

Sync reader mode to github tip ( 8525c6af36d3badbe27c4672a6f2dd99ddb4097f )

Categories

(Toolkit :: Reader Mode, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

Current tip: https://github.com/mozilla/readability/commit/8525c6af36d3badbe27c4672a6f2dd99ddb4097f

Last sync was bug 1425270, which was fa9d8bda48ee574bcffbc19d68b4ca39e1f9036a. Git log:

commit 8525c6af36d3badbe27c4672a6f2dd99ddb4097f (HEAD -> master, upstream/master, upstream/HEAD, origin/master)
Author: Brad Philips
Date:   Fri Mar 2 11:37:16 2018 +0000

    Fix relative URIs given <base> tags (#422)

commit d598baf02b5a7d803074915095891ae17c70feea
Author: Gijs Kruitbosch
Date:   Tue Feb 27 17:36:02 2018 +0000

    Improve URL handling in JSDOMParser and Readability.js
    
    This change ups the required node version to 7.0 because it relies on the builtin url module.
    
    We now pass a url when constructing a jsdom document or JSDOMParser document.
    Because this is an API change, I'm increasing the package version.
    
    Ultimately, I would like to remove the  argument from the readability constructor. It should
    use the documentURI from the document it is passed.

commit 834672ef864191f780a411030bb7c73e86b0aec0
Author: Andres Rey
Date:   Tue Feb 27 14:26:54 2018 +0000

    Return longest text after failing to detect text longer than the configured value (#423)
    
    Save extracted text across attempts and return the longest one when all attempts fail, and add a test case from hukumusume

commit 264b8e8968dc0299d9b18d261a13d00d15baa040
Author: Tom Zöhner
Date:   Tue Jan 30 15:40:49 2018 +0100

    Remove link elements when preparing article for display


Corresponding pull requests:

https://github.com/mozilla/readability/pull/425
https://github.com/mozilla/readability/pull/423
https://github.com/mozilla/readability/pull/422
https://github.com/mozilla/readability/pull/419

Essentially, this fixes:
- inclusion of random stylesheets (though I expect in Firefox's case, the sanitizer would take care of this anyway)
- giving up when there's too little text on the page (this fixes hopefully all of https://bugzilla.mozilla.org/buglist.cgi?quicksearch=prod%3DToolkit%20comp%3Areader%20fail%20article&list_id=14045336 - in the sense that they will now get content. It may still not be the *best* content, but that's going to be the next step, and it's still better than the useless error we currently show)
- relative URI handling when there's a <base> tag that doesn't get included in the reader mode output (e.g. because it's in <head> ).

This should be fairly straightforward to merge.
Comment on attachment 8957156 [details]
Bug 1444082 - sync reader mode to github tip ( 8525c6af36d3badbe27c4672a6f2dd99ddb4097f ),

https://reviewboard.mozilla.org/r/226088/#review232344

Yup, this seems good. Thanks!
Attachment #8957156 - Flags: review?(jhofmann) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/609e4952a46e
sync reader mode to github tip ( 8525c6af36d3badbe27c4672a6f2dd99ddb4097f ), r=johannh
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dd45b23b67c8
Backed out changeset 609e4952a46e for perma-failing android-4.3-arm7-api-16/opt-robocop-3 (RC3) CLOSED TREE
Flags: needinfo?(gijskruitbosch+bugs)
Sigh, similarly-trivial change to a 'this is not an article' testcase as was already in the patch for the toolkit tests, green try (can't easily test locally, hi android robocop...):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=029b428a49894a46671fe20180bcadadbd69eec3&selectedJob=167186428

Relanding in a second.
Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/733da5dbad32
sync reader mode to github tip ( 8525c6af36d3badbe27c4672a6f2dd99ddb4097f ), r=johannh
https://hg.mozilla.org/mozilla-central/rev/733da5dbad32
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Blocks: 1427002
You need to log in before you can comment on or make changes to this bug.