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

fix: regression with unquoted attribute with trailing slash, revert bogus comment minification #1561

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

DylanPiercey
Copy link
Contributor

Description

#1557 made it so that Marko would omit quotes around attributes that did not contain any invalid characters. However this added a regression for the last attribute if it had a trailing slash, ie:

<a rel=external href=http://ebay.com/>
  Go!
</a>

If the href attribute was first, it would actually parse correctly, however the ambiguity above means that the <a> is closed too early which caused the regression.

The fix in this PR is to add a space if the unquoted attribute has a trailing slash.

This PR also reverts the comment minification from that PR. Although there was no regressions with that, and it is objectively smaller output, it does cause html linters and validators to complain. This may appear in a future version of Marko, but we need to re-evaluate.

Checklist:

  • I have read the CONTRIBUTING document and have signed (or will sign) the CLA.
  • I have updated/added documentation affected by my changes.
  • I have added tests to cover my changes.

@codecov
Copy link

codecov bot commented Apr 24, 2020

Codecov Report

Merging #1561 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1561   +/-   ##
=======================================
  Coverage   90.54%   90.54%           
=======================================
  Files         355      355           
  Lines       12769    12769           
=======================================
  Hits        11562    11562           
  Misses       1207     1207           
Impacted Files Coverage Δ
...ges/marko/src/runtime/components/beginComponent.js 100.00% <100.00%> (ø)
...kages/marko/src/runtime/components/endComponent.js 100.00% <100.00%> (ø)
packages/marko/src/runtime/html/AsyncStream.js 89.41% <100.00%> (ø)
packages/marko/src/runtime/html/helpers/attr.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc21820...9550741. Read the comment docs.

@mlrawlings mlrawlings changed the title fix: regression with unquoted attribute with trailing slash fix: regression with unquoted attribute with trailing slash, revert bogus comment minification Apr 24, 2020
@DylanPiercey DylanPiercey merged commit 5c26b9b into master Apr 24, 2020
@DylanPiercey DylanPiercey deleted the unquoted-attr-regression branch April 24, 2020 23:41
DylanPiercey added a commit that referenced this pull request Apr 27, 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.

1 participant