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(html_tag): encode url value of url-related attributes and skip escape/encode <style> #96

Merged
merged 9 commits into from Sep 21, 2019

Conversation

@curbengh
Copy link
Contributor

commented Sep 17, 2019

address #94 (comment) to support url value in data-* attributes (e.g. data-url, data-src)
cc @dailyrandomphoto

test case is already covered by

  it('encode url', () => {
    htmlTag('img', {
      src: 'http://foo.com/bár.jpg'
    }).should.eql('<img src="http://foo.com/b%C3%A1r.jpg">');
  });
curbengh added 6 commits Sep 15, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

@dailyrandomphoto
any other attribute that I might've missed?

@coveralls

This comment has been minimized.

Copy link

commented Sep 17, 2019

Coverage Status

Coverage decreased (-1.4%) to 95.079% when pulling 02e1e52 on curbengh:html-tag-url into 6155112 on hexojs:master.

@curbengh curbengh force-pushed the curbengh:html-tag-url branch from 4f294e9 to 2c91312 Sep 17, 2019
This was referenced Sep 17, 2019
@curbengh curbengh changed the title fix(html_tag): encode value of data-url as url fix(html_tag): encode url value of url-related attributes Sep 17, 2019
@dailyrandomphoto

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

@dailyrandomphoto
any other attribute that I might've missed?

I have tested some cases and found no issues.

@curbengh

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

Just found an edge case for responsive image

<img srcset="elva-fairy-320w.jpg 320w,
             elva-fairy-480w.jpg 480w,
             elva-fairy-800w.jpg 800w"
     sizes="(max-width: 320px) 280px,
            (max-width: 480px) 440px,
            800px"
     src="elva-fairy-800w.jpg" alt="Elva dressed as a fairy">

🤔 Let's see possible approaches:

  1. split by space and encode each part
  2. regex match string that ends with an image extension (I saw the regex pattern somewhere in hexo, can't find it), then encode it.
@SukkaW

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

@curbengh

/<img [^>]*src=['"]([^'"]+)([^>]*>)/gi

This is the <img> tag pattern used in open_graph() helper.

https://github.com/hexojs/hexo/blob/19ac152ac3fd708b1860244187442c587be73031/lib/plugins/helper/open_graph.js#L62

@curbengh

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

/<img [^>]src='"([^>]>)/gi

That's actually is the first regex I tried, but it's not applicable here because html_tag is not parsing a html. Essentially, the goal is just to avoid encoding the space in elva-fairy-800w.jpg 800w.


Anyway, the latest fix, while seems overcomplicating stuff, it's to preserve new lines. Frankly, I don't know if anyone actually wants it (new line) 😅 . Personally, I would just go for handleSpaceNewLineMinify() approach of following examples.

function handleSpace(str) {
  return encodeURI(str).replace(/(%20)/g, ' ');
}

function handleSpaceNewLine(str) {
  return encodeURI(str).replace(/(%20|%0A)/g, ' ');
}

function handleSpaceNewLineMinify(str) {
  return encodeURI(str).replace(/%20/g, ' ').replace(/,(%0A|%0D%0A)*\s*/g, ',');
}

let data = `fóo.jpg 320w,
        /foo/bár.jpeg 480w,
        default.png`

console.log(handleSpace(data))
// f%C3%B3o.jpg 320w,%0A        /foo/b%C3%A1r.jpeg 480w,%0A        default.png

console.log(handleSpaceNewLine(data))
// f%C3%B3o.jpg 320w,         /foo/b%C3%A1r.jpeg 480w,         default.png

console.log(handleSpaceNewLineMinify(data))
// f%C3%B3o.jpg 320w,/foo/b%C3%A1r.jpeg 480w,default.png
@curbengh curbengh requested a review from hexojs/core Sep 20, 2019
This was referenced Sep 20, 2019
curbengh added 3 commits Sep 20, 2019
and skip escape/encode <style> tag
lib/html_tag.js Outdated Show resolved Hide resolved
@curbengh curbengh changed the title fix(html_tag): encode url value of url-related attributes fix(html_tag): encode url value of url-related attributes and skip escape/encode <style> Sep 21, 2019
@curbengh curbengh requested a review from SukkaW Sep 21, 2019
@SukkaW
SukkaW approved these changes Sep 21, 2019
@curbengh curbengh merged commit 9d96e6d into hexojs:master Sep 21, 2019
2 of 3 checks passed
2 of 3 checks passed
coverage/coveralls Coverage decreased (-1.4%) to 95.079%
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@curbengh curbengh deleted the curbengh:html-tag-url branch Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.