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

Move resolve step last in u-* parsing #170

Merged
merged 1 commit into from
Aug 1, 2018

Conversation

Zegnat
Copy link
Member

@Zegnat Zegnat commented Apr 20, 2018

This matches the proposed parsing change from microformats/microformats2-parsing#10.

Note that one old test had to be updated, because it tested for textContent being returned as is. And now we resolve strings gotten from there too.

If I am reading the URL spec right though, the URL we are returning isn’t actually a valid URL. Spaces are in the path percent-encode set so should be replaced with %20 … I think. Ideas?

@aaronpk
Copy link
Member

aaronpk commented Aug 1, 2018

I think I'm in favor of merging this as is, even if it still returns invalid URLs, because it won't return anything more broken than the current version. I do think it should properly escape URLs in the parsed result though, since that's what would happen if you included a link in an <a> tag in a browser that included spaces. (The browser converts those to %20.

@aaronpk aaronpk merged commit c52fd0d into microformats:master Aug 1, 2018
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