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

Updated regexp to properly handle dot in attribute name #204

Closed
wants to merge 1 commit into from

Conversation

rzhornyk
Copy link
Contributor

No description provided.

@rzhornyk rzhornyk changed the title #203 Updated regexp to properly handle dot in attribute name Updated regexp to properly handle dot in attribute name Sep 23, 2020
@rzhornyk rzhornyk mentioned this pull request Sep 23, 2020
@tbranyen
Copy link
Owner

Good find, turns out diffHTML is missing a bunch of valid characters in HTML attribute names. I'm traveling right now, but I would suspect that we need to add what isn't valid, vs what is in this case.

@rzhornyk
Copy link
Contributor Author

So as I see according to spec https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
next regexp can be used:

/\b([^\s\x00-\x1F"'>\/=\uFDD0-\uFDEF\uFFFE\uFFFF]*)\s*(=\s*("([^"]+)"|'([^']+)'|(\S+)))?/ig

What do you think?

@tbranyen
Copy link
Owner

I tried this regex out and while it looks almost identical to the one we currently use, it is causing 1-more tests to hang indefinitely. Very strange. Will investigate more and try to get this landed soon.

@tbranyen
Copy link
Owner

tbranyen commented Oct 11, 2020

Found numerous tests which fail, but this one failed first and seems relevant to this issue (attribute matching). Seems that attribute matching was used for interpolated dynamic value as well, and the previous regex was less strict allowing for a __STYLE_VALUE__ used as interpolation tokens.

it('will interpolate value-less attributes', function() {
  const checked = 'checked';
  const input = html`<input type="checkbox" ${checked}>`;

  equal(input.attributes.checked, 'checked');
});

Which causes an infinite loop:

[ '',
  '',
  undefined,
  undefined,
  undefined,
  undefined,
  undefined,
  index: 31,
  input: 'type="checkbox" __DIFFHTML__0__',
  groups: undefined ]

I think this regex is too complex and we need to simplify it for the needs of the parser, which is meant to operate more loosely. We do have a strict mode, and could potentially toggle the regex between modes.

@tbranyen
Copy link
Owner

Declining in favor of #207 which updates using the regex matching provided in your comment above @rzhornyk. Thanks for bringing up the issue!

@tbranyen tbranyen closed this Nov 12, 2020
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

Successfully merging this pull request may close these issues.

2 participants