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: image tag format throw error #73

Merged
merged 4 commits into from
Nov 24, 2018
Merged

Conversation

xiaomingplus
Copy link
Contributor

fix #72

Hi,I don't think image should a valid tag in html, so I remove the void tag from html-void-elements.

here are the expected examples:

input:

<template>
 <div>
   <image src="http://google.com" />
 </div>
</template>

output:

<template>
  <div>
    <image src="http://google.com" />
  </div>
</template>

or,input:

    <image src="http://google.com"></image>

output:

   <image src="http://google.com"></image>

or ,input :

<image src="">

output:

<image src="" />

packages/prettyhtml-hast-to-html/lib/index.js Outdated Show resolved Hide resolved
packages/webparser/src/html_tags.ts Outdated Show resolved Hide resolved
packages/prettyhtml-hast-to-html/lib/index.js Outdated Show resolved Hide resolved
@StarpTech
Copy link
Member

StarpTech commented Nov 22, 2018

Hi @xiaomingplus the PR looks good except that the code isn't formatted with prettier and you didn't add any tests.

@xiaomingplus
Copy link
Contributor Author

hello, @StarpTech ,I thought about it for a moment, image should really be treated as an void element instead of a custom element.
Here are the MDN intro https://developer.mozilla.org/en-US/docs/Web/HTML/Element/image

so,I recommit a pr for adding image as a void element.

Can you review this?

thanks.

@StarpTech
Copy link
Member

@xiaomingplus We should handle it as a custom element because (quote below) and the fact that the tag is Obsolete and Non-standard

Creating an element without a src attribute results in an HTMLElement object with the local element name "image".

@xiaomingplus
Copy link
Contributor Author

Ok,I agree it,I'll give a pull request for that

@xiaomingplus
Copy link
Contributor Author

if we treat image as a custom component, the custom default format code can't through the vetur default template check..
image

image

Can you give some advise?

@StarpTech
Copy link
Member

@xiaomingplus I also double check it. The browsers will replace image with img even when it contains no src attribute. image can't be used as element selector in vue https://codesandbox.io/s/jpl947mrry. Since it collides with the ecosystem we should treat image as a self-closing tag. What a tedious case 😄

@xiaomingplus
Copy link
Contributor Author

xiaomingplus commented Nov 24, 2018

Actually,I use vue for weex(a react native like but using vue) development

Weex  provide a image component, https://weex.apache.org/references/components/image.html

So,treat image as self-closing tag meaning?Can you give some expected output?

@StarpTech
Copy link
Member

StarpTech commented Nov 24, 2018

@xiaomingplus for me it looks like that weex is no real html framework. The markup won't reach a native browser.

However, Weex is not a front-end framework. Indeed, front-end frameworks are just the syntax layer or DSL (Domain-specific Language) of Weex, they are decoupled from native render engines. In another word, Weex does not rely on any specific front-end frameworks. With the evolution of the technology, Weex can integrate more widely used front-end frameworks as well.

Therefore I don't want to support it in first instance.

This is an important info:

Note: The HTML spec defines as a synonym for while parsing HTML. This specific element and its behavior only apply inside SVG documents or inline SVG.
https://developer.mozilla.org/en-US/docs/Web/SVG/Element/image

Try this in prettyhtml, it behaves correctly:

<svg>
<image src="http://google.com"></image>
</svg>
<image src="http://google.com"></image>

output

<svg>
<image src="http://google.com"></image>
</svg>
<image src="http://google.com">

Finally there is no change needed. image should be never used as an element selector out of svg. I'm fine with that.

@StarpTech
Copy link
Member

But I will prepare a fix so that no invalid markup is generated.

@xiaomingplus
Copy link
Contributor Author

but,if you include your output in div,it will throw a error.

input :

<div>
<svg>
<image src="http://google.com"></image>
</svg>
<image src="http://google.com">
</div>

it throw a error.

I think you can merge the pull request simply. :)

@StarpTech
Copy link
Member

Hi @xiaomingplus indeed the PR looks good I've worked on the same solution. I missing some test I will add them to the PR.

@xiaomingplus
Copy link
Contributor Author

Thanks~

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.

foramt image tag throw error
2 participants