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

Error parsing input element's pattern attribute #306

Closed
jsumners opened this issue Jun 9, 2016 · 9 comments
Closed

Error parsing input element's pattern attribute #306

jsumners opened this issue Jun 9, 2016 · 9 comments

Comments

@jsumners
Copy link
Contributor

jsumners commented Jun 9, 2016

Given:

<input type="text"
           name="username"
           placeholder="username"
           spellcheck="false"
           autocomplete="off"
           autocorrect="off"
           pattern="[\w0-9]+">

Marko will fail with an error:

Error(s) in template:

  1. [templates/views/askForUsername.marko:16:20] Invalid string ("[\w0-9]+"): SyntaxError: Unexpected token w

This is "fixable" by pattern="[\\w0-9]+, but that doesn't follow the HTML spec -- https://html.spec.whatwg.org/multipage/forms.html#attr-input-pattern . In other words, the value should be a regular expression that can be directly substituted into a JavaScript regular expression literal, e.g. /[\w0-9]+/.

@patrick-steele-idem
Copy link
Contributor

patrick-steele-idem commented Jun 9, 2016

An attribute value needs to always follow the rules of JavaScript parsing. Unfortunately, that does introduce some deviation from the HTML spec. However, Marko is not using an HTML parser, but rather the HTMLJS parser, and the HTMLJS parser always parses an attribute value as a JavaScript expression. We would not want to special case various attributes since that would makes things problematic.

It might be helpful to think of it has having to write JavaScript code to set an HTML value:

var input = document.createElement('input');
input.setAttribute('pattern', '[\\w0-9]+'; 
// Note that the backslash character needs to be escaped.

Hope that answer satisfies your issue, but please let me know if not.

@jsumners
Copy link
Contributor Author

jsumners commented Jun 9, 2016

I understand the underlying issue. But my opinion is that when writing HTML within the template we shouldn't have to remember "no, this isn't really HTML, it's an escaped JavaScript string."

I think a solution to this, and probably other cases, would be to have a <raw></raw> Marko directive.

@patrick-steele-idem
Copy link
Contributor

I understand the underlying issue. But my opinion is that when writing HTML within the template we shouldn't have to remember "no, this isn't really HTML, it's an escaped JavaScript string."

I definitely understand where you are coming from. To keep things simple we want to only support one way of parsing things and found that JavaScript expressions were a much better fit for attribute values. The only thing that you need to remember is that the right-hand side of an attribute value is always a JavaScript expression (at least it is consistent).

I think a solution to this, and probably other cases, would be to have a Marko directive.

We could, in theory, introduce a new syntax to break out of the rules of standard JavaScript parsing for an attribute value to support a "raw" value, but that would introduce more complexity and likely very little gain. Hypothetically, we could support triple quotes (borrowed from CoffeeScript) or something random:

<input type="text"
           name="username"
           placeholder="username"
           spellcheck="false"
           autocomplete="off"
           autocorrect="off"
           pattern="""[\w0-9]+""">

Do you think that is worth considering? I'm leaning towards no, because I want to keep the syntax rules simple, but I am open to the idea if there is enough interest.

@jsumners
Copy link
Contributor Author

jsumners commented Jun 9, 2016

I suppose if extra syntax is added, what about something closer to what is already present for skipping HTML escaping?

<input#foo class=${a: true, b: false} pattern=!"[\w0-9]">

It has the bonus of being almost like a CSS attribute selector.

@patrick-steele-idem
Copy link
Contributor

patrick-steele-idem commented Jun 9, 2016

/cc @philidem @mlrawlings @tindli Thoughts?

@mlrawlings
Copy link
Member

mlrawlings commented Jun 10, 2016

I'm not sure adding something that deviates further from html (=! or anything else) to get back to html really makes sense.

My biggest concern is that someone new to marko may not realize wha the issue is with the error presented. Given that it's a pretty standard js error maybe this isn't really a concern. It might be worth it if we could point out this difference in parsing when it comes up though.

I do understand not wanting to double escape. It's a pain when you have to use strings to pass to new RegExp() too. For this specific case, I think it would be nice to be able to use a js regex as the value (we'd have to remove the beginning and ending / after calling .toString()) :

<input pattern=/[\w0-9]+/>

Are there other attributes other than pattern that might parse unexpectedly?

@jsumners
Copy link
Contributor Author

My point is "who knows?" Anyone can stick pretty much anything they want in a data attribute. And future spec graced attributes could have anything as well.

My use case is a simple fix on my end (other than remembering when I come back to the project in 6 months why there is an extra \). I just think that it would be wise to provide a template language specific method for dealing with such scenarios so that someone who knows the language can read the intention.

@mlrawlings
Copy link
Member

mlrawlings commented Jun 10, 2016

If we want to change the way strings are parsed, the only thing I think we should consider is changing the default. The only change worth making is one that allows you to paste in valid HTML without any changes, otherwise it's easier to explain "strings are JS strings" rather than introducing additional syntax.

However, I'm not sure this is a good idea. As to what other attributes this might affect, I guess the better question is where does the HTML attribute string spec deviate from the JS string spec? Is it just that \ needs to be escaped or are there other differences?

@mlrawlings
Copy link
Member

mlrawlings commented Jun 10, 2016

One (probably bad) thought: attempt to parse as JS value, and if that fails, fall back to the HTML way of parsing attributes.

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