Skip to content

Fix plain text links in citations - #1022

Open
aakashsbhatia2 wants to merge 3 commits into
mozilla:mainfrom
aakashsbhatia2:fix/plain-text-links
Open

Fix plain text links in citations#1022
aakashsbhatia2 wants to merge 3 commits into
mozilla:mainfrom
aakashsbhatia2:fix/plain-text-links

Conversation

@aakashsbhatia2

Copy link
Copy Markdown

This fixes issue - #1016

It seems like the issue is in the scoring algorithm. The end links are scored higher than the article content causing it to take precedence. I updated _getLinkDensity() to consider urls not wrapped in <a> tags as well. Added the source as a test case

@gijsk gijsk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

This looks interesting and the change makes sense to me. My main question is whether we really need the argument - do we not want to count the plaintext links for all _getLinkDensity callers? If not, why not? :-)

Comment thread Readability.js
**/
_getLinkDensity(element) {
_getLinkDensity(element, includePlainTextUrls) {
var textLength = this._getInnerText(element).length;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's maybe cache the return of getInnerText which may not be cheap for large nodes, given we now potentially reuse it further down?

@gijsk

gijsk commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Oh, and one minor thing - can we strip scripts from the input HTML to avoid accidentally running them when checking testcases? Thank you!

(if the generate testcase script doesn't do this, it wouldn't hurt to change that... though obviously that's a separate problem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants