Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cloning attributes to use setAttributeNode to fix #859 #889

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions JSDOMParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@
getEncodedValue: function() {
return encodeHTML(this._value);
},

// Cheat horribly. This is fine for our usecases.
cloneNode: function() {
return this;
},
};

var Comment = function () {
Expand Down Expand Up @@ -777,6 +782,10 @@
this.attributes.push(new Attribute(name, value));
},

setAttributeNode: function (node) {
this.setAttribute(node.name, node.value);
},

removeAttribute: function (name) {
for (var i = this.attributes.length; --i >= 0;) {
var attr = this.attributes[i];
Expand Down
13 changes: 2 additions & 11 deletions Readability.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ Readability.prototype = {
} else if (this._hasSingleTagInsideElement(node, "DIV") || this._hasSingleTagInsideElement(node, "SECTION")) {
var child = node.children[0];
for (var i = 0; i < node.attributes.length; i++) {
child.setAttribute(node.attributes[i].name, node.attributes[i].value);
child.setAttributeNode(node.attributes[i].cloneNode());
}
node.parentNode.replaceChild(child, node);
node = child;
Expand Down Expand Up @@ -650,16 +650,7 @@ Readability.prototype = {
replacement.readability = node.readability;

for (var i = 0; i < node.attributes.length; i++) {
try {
replacement.setAttribute(node.attributes[i].name, node.attributes[i].value);
} catch (ex) {
/* it's possible for setAttribute() to throw if the attribute name
* isn't a valid XML Name. Such attributes can however be parsed from
* source in HTML docs, see https://github.com/whatwg/html/issues/4275,
* so we can hit them here and then throw. We don't care about such
* attributes so we ignore them.
*/
}
replacement.setAttributeNode(node.attributes[i].cloneNode());
}
return replacement;
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"title": "Dungeons & Dragons and Burgers: ‘Really Bad Outcomes’ When We Don’t Grasp Fractions",
"byline": "•",
"dir": "ltr",
"lang": "en",
"excerpt": "People rely too heavily on anecdotal evidence when looking at complex problems. Author and UVA scientist James Zimring says the solution to better understanding lies in understanding fractions.",
"siteName": "UVA Today",
"publishedTime": "2022-05-27T08:00:00-04:00",
"readerable": true
}
12 changes: 12 additions & 0 deletions test/test-pages/virginia-edu-broken-attributes/expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<div id="readability-page-1" class="page">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't actually very good, but it addresses the invalid markup breaking us entirely.

<div 2="">
<p>Zimring said humans have still not learned to look at problems holistically.</p>
<p>“Setting aside the controversy regarding Dungeons &amp; Dragons, people now claim that dark music and/or violent video games increase teen suicide, but it seems we haven’t learned our lesson because instead of saying, ‘All right, well, let’s look at these factors and take into account these probabilistic determinations,’ people jump right to a causal conclusion,” he said. “This is one example of a human tendency to ignore the whole fraction.”</p>
<p>Zimring offers several other examples in his book to prove this point. One is the sad tale of a massive marketing flop by A&amp;W Restaurants, which thought it had a home run.</p>
<p>The burger joint set out to take on one of MacDonald’s’ signature sandwiches, the Quarter Pounder. A&amp;W’s burger tasted better in blind taste tests. It cost less. It was bigger. They called it the “Third Pound Burger.”</p>
<p>It bombed. A market postmortem would reveal why. “The best they could determine was that people didn’t want to buy it because they thought that a third of a pound was less than a quarter pound because three is less than four,” Zimring said.</p>
<p>However, this human shortcoming of failing to consider statistical data when trying to understand a situation does have a good side, he said. It lies in something called heuristics – a cognitive framework humans rely upon to make quick, though sometimes imperfect, decisions.</p>
<p>“You can’t pay attention to every leaf blowing in the wind as you’re driving,” Zimring said by example. “But if one of those leaves suddenly explodes and bursts into flames, you should probably pay attention to that. We use heuristics to allow our brains, which are limited, to process information quickly enough that we can actually navigate the world in real time.</p>
<p>“We sacrifice the more logical reasoning systems for these shortcuts because they let us get by and they usually work,” Zimring said. “It's just that when they fail, they can fail disastrously.”</p>
</div>
</div>
Loading