Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

DOMException with an unclosed attribute #68

Open
pamelafox opened this issue Dec 1, 2014 · 4 comments
Open

DOMException with an unclosed attribute #68

pamelafox opened this issue Dec 1, 2014 · 4 comments

Comments

@pamelafox
Copy link

With HTML like this:

<!DOCTYPE HTML>
<html>
    <head>
        <title>Challenge: A picture-perfect trip</title>
        <meta charset="utf-8">
    </head>
    <body>
        <h1>The perfect trip</h1>

        <p>I would see scenes like...</p>
        <img src="https://www.kasandbox.org/programming-images/landscapes/beach-waves-at-sunset.png"height="206" alt= "a beautiful sunset at the beach>

        <p>And animals like...</p>
        <img src="https://www.kasandbox.org/programming-images/animals/cheetah.png" height="206">

        <p>And eat food like...</p>
        <img src="https://www.kasandbox.org/programming-images/food/cake.png" height="206">

    </body>
</html>

The following error occurs:
"Failed to execute 'createAttribute' on 'Document': The qualified name provided ('https:') has an empty local name."

That is because it thinks that the quote sign ends the attribute, and thus the thing after it, https, is an attribute name, and then I think it confuses it with a namespaced tag due to the colon.

It would be nice if it could fail better if possible.
On Thimble, this results in a silent error, by the way. On KA, we are catching this and outputting "Something's wrong with the HTML, but we're not sure what."
@pamelafox
Copy link
Author

Related issue that causes a DOM exception due to a colon in what's interpreted as a class name:

<strong class:"apples">apples</strong>

@Pomax
Copy link
Contributor

Pomax commented Dec 1, 2014

yeah those look like cases slowparse doesn't trip on atm. The first will be rather tricky, since technically the first detectable error in your example code is that http://... is not a valid attribute name, so it should be breaking there. Unfortunately, while super obvious to humans, that missing quote symbol is followed by pretty much 100% legal attribute content. Worth thinking about, but will be hard to deal with.

The second case should already be caught so if it's not, let's add a unit test and fix it. Colons are not allowed in attribute names so that should have been flagged.

@pamelafox
Copy link
Author

I ran into <strong class:"apples">apples</strong> again this weekend at a workshop. Would that be easy to fix? (I could give it a go if you don't have the resources, any hints you want to drop about where to poke in the code would be appreciated).

@Pomax
Copy link
Contributor

Pomax commented Feb 10, 2015

we're doing a sprint on some unrelated code this week so I won't have time to look at it, but I can definitely point you at where things could be fixed: the trick here is that technically class: here could be considered a namespace, which means we might be able to catch it in this codepath: https://github.com/mozilla/slowparse/blob/gh-pages/slowparse.js#L1164 (I'm kind of surprised it didn't, that feels like it's a bug!).

Another thing we could do is check to make sure that IF it's a legal namespace, the subsequent character is a valid word character; in this case the next character is a double quote, which would cause an error.

It's probably worth first adding a test for this to the battery of tests over in https://github.com/mozilla/slowparse/blob/gh-pages/test/test-slowparse.js with your code, and a requirement that slowpase should fail on it, so that it becomes easier to figure out which code that tests "travels through".

@pamelafox pamelafox mentioned this issue Feb 24, 2015
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Pomax @pamelafox