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

Allow regular expression for an HTML attribute value #386

Closed
tinovyatkin opened this issue Oct 30, 2016 · 4 comments
Closed

Allow regular expression for an HTML attribute value #386

tinovyatkin opened this issue Oct 30, 2016 · 4 comments
Assignees
Labels
good first issue Small tasks that would be good for first time contributors type:feature A feature request
Milestone

Comments

@tinovyatkin
Copy link
Contributor

Bug Report

Compiler throws an error on valid RegEx expressions in HTML5 pattern attribute for input elements.

Context

Given following code example

<input type="text" pattern="\w{2,20}" />

Expected Behavior

Compile normally, as it valid attribute value

Actual Behavior

Throws Error
'Invalid string ("\\w{1,30}"): SyntaxError: Unexpected token w in JSON at position 2'

Additional Info

Your Environment

  • Version used: "marko": "^4.0.0-beta.1"
  • Environment name and version: node.js 7.0
  • Operating System and version (desktop or mobile): MacOS

Stack Trace

ERR! [10:40:05] [Marko] 1) [src/marko/vip-puj.marko:111:88] Invalid string ("\w{1,30}"): SyntaxError: Unexpected token w in JSON at position 2
ERR! [10:40:05] [Marko] 
ERR! [10:40:05] [Marko]     at handleErrors (/Users/tino/Sites/transfers.do/node_modules/marko/compiler/Compiler.js:72:21)
ERR! [10:40:05] [Marko]     at Compiler.compile (/Users/tino/Sites/transfers.do/node_modules/marko/compiler/Compiler.js:127:9)
ERR! [10:40:05] [Marko]     at _compile (/Users/tino/Sites/transfers.do/node_modules/marko/compiler/index.js:89:33)
ERR! [10:40:05] [Marko]     at doLoad (/Users/tino/Sites/transfers.do/node_modules/marko/runtime/loader/index.js:163:42)
ERR! [10:40:05] [Marko]     at compileMarkoFile (/Users/tino/Sites/transfers.do/tasks/marko.js:45:26)
ERR! [10:40:05] [Marko]   errors: 
ERR! [10:40:05] [Marko]    [ CompileError {
ERR! [10:40:05] [Marko]        context: [Object],
ERR! [10:40:05] [Marko]        node: undefined,
ERR! [10:40:05] [Marko]        message: 'Invalid string ("\\w{1,30}"): SyntaxError: Unexpected token w in JSON at position 2',
ERR! [10:40:05] [Marko]        code: 'INVALID_STRING',
ERR! [10:40:05] [Marko]        pos: [Object],
ERR! [10:40:05] [Marko]        endPos: [Object] } ] }
@patrick-steele-idem
Copy link
Contributor

Thanks for opening the issue @tinovyatkin, but that is the expected behavior since Marko is using the JavaScript rules for parsing a String attribute value. This means that JavaScript escape sequences starting with the \ character must be valid JavaScript escape sequences. In your case you need to escape the backslash character so that the JavaScript parser doesn't treat it as a JavaScript String escape sequence:

<input type="text" pattern="\\w{2,20}" />

While this is not a bug in Marko, I admit that the escaping of those characters is less than ideal. What are your thoughts on enhancing Marko to allow a JavaScript regular expression for the attribute value?:

<input type="text" pattern=/\w{2,20}/ />

(we could update the compiler so that the output would be as expected)

@tinovyatkin
Copy link
Contributor Author

tinovyatkin commented Oct 30, 2016

I think accepting native RegEx as value will looks logical and very appropriate!

@tinovyatkin
Copy link
Contributor Author

Also, I'm not sure that this pattern attribute will work in same way in all browsers after escaping \\, but can't find related spec thought...

@mlrawlings
Copy link
Member

mlrawlings commented Oct 30, 2016

We should do this. It's come up before.

It should be as simple as adding the following to the escapeXmlAttr function:

if(attr instanceof RegExp) {
    let str = attr.toString();
    return str.slice(1, str.lastIndexOf('/'));
}

The only thing is that html style regex don't support flags, so I'm thinking we just drop them?

@patrick-steele-idem patrick-steele-idem added good first issue Small tasks that would be good for first time contributors type:feature A feature request labels Nov 9, 2016
@patrick-steele-idem patrick-steele-idem changed the title Compiler error on RegEx at HTML5 pattern attribute Allow regular expression for an HTML attribute value Nov 9, 2016
@patrick-steele-idem patrick-steele-idem added this to the 4.0 milestone Nov 9, 2016
@patrick-steele-idem patrick-steele-idem self-assigned this Nov 10, 2016
patrick-steele-idem added a commit that referenced this issue Nov 10, 2016
Fixes #386 - Allow regular expression for an HTML attribute value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Small tasks that would be good for first time contributors type:feature A feature request
Projects
None yet
Development

No branches or pull requests

3 participants