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

jpg/webp/png may appear at alt attribute cause a mistake #689

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

shfshanyue
Copy link
Contributor

@shfshanyue shfshanyue commented Apr 14, 2021

jpg/webp/png may appear at alt attribute, will cause a mistake

<img
  class="lazyload inited loaded"
  data-src="https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/0579d17015b145a88dd93992c6447d7d~tplv-k3u1fbpfcp-watermark.jpg"
  alt="performance.jpg"
>

expected

<img
  class="lazyload inited loaded"
  data-src="https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/0579d17015b145a88dd93992c6447d7d~tplv-k3u1fbpfcp-watermark.jpg"
  alt="performance.jpg"
  src="https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/0579d17015b145a88dd93992c6447d7d~tplv-k3u1fbpfcp-watermark.jpg"
>

actually

<img
  class="lazyload inited loaded"
  data-src="https://p3-juejin.byteimg.com/tos-cn-i-k3u1fbpfcp/0579d17015b145a88dd93992c6447d7d~tplv-k3u1fbpfcp-watermark.jpg"
  alt="performance.jpg"
  src="performance.jpg"
>

@shfshanyue shfshanyue changed the title fix: alt is possible to include image extension jpg/webp/png may appear at alt attribute cause a mistake Apr 14, 2021
@gijsk
Copy link
Contributor

gijsk commented Apr 14, 2021

Thanks for your PR! The code change looks good - do you think you'd be able to add a small testcase for this? You can create a new directory under the test/test-pages/ bit of the repo. In that directory, add a small source.html that has the pattern that is breaking before your changes, and then run node test/generate-testcase.js <your-directory-name> to generate the expected output and metadata, and then check it is OK.

@shfshanyue
Copy link
Contributor Author

Thanks for your PR! The code change looks good - do you think you'd be able to add a small testcase for this? You can create a new directory under the test/test-pages/ bit of the repo. In that directory, add a small source.html that has the pattern that is breaking before your changes, and then run node test/generate-testcase.js <your-directory-name> to generate the expected output and metadata, and then check it is OK.

it's a good idea

@gijsk
Copy link
Contributor

gijsk commented Apr 15, 2021

This looks great, thank you!

@gijsk gijsk merged commit e197236 into mozilla:master Apr 15, 2021
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