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

Uncaught DOMException on https://losst.ru/obnovlenie-debian-9 #392

Closed
rNeomy opened this issue Sep 2, 2017 · 8 comments
Closed

Uncaught DOMException on https://losst.ru/obnovlenie-debian-9 #392

rNeomy opened this issue Sep 2, 2017 · 8 comments

Comments

@rNeomy
Copy link

rNeomy commented Sep 2, 2017

I am getting

Uncaught DOMException: Failed to execute 'setAttribute' on 'Element': 'adsbygoogle.js"' is not a valid attribute name.
    at Readability._setNodeTag (Readability.js:477:19)
    at Readability._grabArticle (Readability.js:774:25)
    at Readability.parse (Readability.js:2062:31)
    at 

which refers to

  _setNodeTag: function (node, tag) {
    this.log("_setNodeTag", node, tag);
    if (node.__JSDOMParser__) {
      node.localName = tag.toLowerCase();
      node.tagName = tag.toUpperCase();
      return node;
    }

    var replacement = node.ownerDocument.createElement(tag);
    while (node.firstChild) {
      replacement.appendChild(node.firstChild);
    }
    node.parentNode.replaceChild(replacement, node);
    if (node.readability)
      replacement.readability = node.readability;

    for (var i = 0; i < node.attributes.length; i++) {     /*the next line*/
      replacement.setAttribute(node.attributes[i].name, node.attributes[i].value);
    }
    return replacement;
  },
@gijsk
Copy link
Contributor

gijsk commented Sep 12, 2017

Can you provide more detailed STR? If I open the page in Firefox and enter reader mode, it works. What are you doing differently?

@rNeomy
Copy link
Author

rNeomy commented Sep 13, 2017

I am porting this to Chrome. To regenerate you can follow this

  1. Open the page in Chrome
  2. Open dev tools
  3. Paste the latest Readability.js
  4. paste this code
var loc = document.location;
var uri = {
  spec: loc.href,
  host: loc.host,
  prePath: loc.protocol + '//' + loc.host,
  scheme: loc.protocol.substr(0, loc.protocol.indexOf(':')),
  pathBase: loc.protocol + '//' + loc.host + loc.pathname.substr(0, loc.pathname.lastIndexOf('/') + 1)
};

var documentClone = document.cloneNode(true);
var article = new Readability(uri, documentClone).parse();

var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
iframe.contentDocument.open();
iframe.contentDocument.write(article.content);
iframe.contentDocument.close();

which causes:

Uncaught DOMException: Failed to execute 'setAttribute' on 'Element': 'adsbygoogle.js"' is not a valid attribute name.
    at Readability._setNodeTag (<anonymous>:477:19)
    at Readability._grabArticle (<anonymous>:774:25)
    at Readability.parse (<anonymous>:2062:31)
    at <anonymous>:11:51

@gijsk
Copy link
Contributor

gijsk commented Jul 2, 2018

I can no longer reproduce this with the latest version of Readability and Chrome, so I assume this has been fixed since. :-)

@gijsk gijsk closed this as completed Jul 2, 2018
@MrOrz
Copy link

MrOrz commented Jan 5, 2019

@gijsk I can reproduce the Uncaught DOMException: Failed to execute 'setAttribute' on 'Element' error on different pages.

Here are the reproduction steps:

  1. Visit https://opinion.udn.com/opinion/story/10124/3561413 on Google Chrome
  2. Open Devtool JS console
  3. paste in latest readability.js
  4. execute new Readability(location.href, document).parse();

Expected: article being parsed
Actual: thown error Uncaught DOMException: Failed to execute 'setAttribute' on 'Element': '0' is not a valid attribute name.

I have a bunch of URLs that can be used to reproduce the above behavior:
https://docs.google.com/spreadsheets/d/1TwDZmocHxlcSPm2zQoGJF167Uj9RNgqHREEx3PEa580/edit?usp=sharing

Apparently it's caused by malformed HTML, but it should not crash the entire readability.

@gijsk
Copy link
Contributor

gijsk commented Jan 7, 2019

Well, this is fun... I filed whatwg/html#4275 for the inconsistency between how HTML is parsed and how the DOM APIs validate exactly the same thing. I should have a patch for this using the workaround mentioned there (using element.attributes.setNamedItem() instead of element.setAttribute()).

@gijsk
Copy link
Contributor

gijsk commented Jan 7, 2019

Yum, and cloneNode() is not supported for Attr in jsdom... jsdom/jsdom#2478

@gijsk
Copy link
Contributor

gijsk commented Jan 7, 2019

OK, so maybe the simpler solution is just to ignore those additional attributes that we can't even set; it's not like we need them for anything...

@gijsk
Copy link
Contributor

gijsk commented Jan 7, 2019

OK, so PR is up, as well as issues found in jsdom and HTML/DOM as a result. Thanks @MrOrz for the clear steps to reproduce!

@gijsk gijsk closed this as completed in 30f9670 Jan 7, 2019
1257mp pushed a commit to 1257mp/readability that referenced this issue Jul 19, 2019
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

No branches or pull requests

3 participants