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

The behavior about width or height attribute of <img /> is not diffrent between vue and original html #8780

Closed
zh-lx opened this issue Jul 14, 2023 · 15 comments · Fixed by #8781
Labels
need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.

Comments

@zh-lx
Copy link
Contributor

zh-lx commented Jul 14, 2023

Vue version

3.3.4

Link to minimal reproduction

https://play.vuejs.org/#eNqNUctOwzAQ/JXIZxJXlFNVUAH1AAdAwDGXNFlSF79kr5tKVf+dtVvSUB7i5p1Z78zObtm1tcU6AJuwqa+dsJh5wGCvSj3le4CeVCAoKysEqrJsKlSbeVdflmyJaP2E867r4px85YvaKG7DQoqaC1W14DkRhdVtybJONLikb+ejkd1QzZNQP5ydMaGscZiryhYrbzQZ20bJ8kD4kk2yhESMBsf6aKNuNH1rQIq1KzQg11bxGbVxFzQKBXlj1GxcjIsL3giPQ7gAr/KFM50HR0NKdjaQ4QSuweUOdAMO3H9lT74NpU+ob/JRfVfqHYWCvjb6TbQnkVDSVkhwjxaF0V+jqaQ03X3C0AXod6mXUL//gK883SPu9OQgORvsj5VrAff0/OUBNvTuSWWaIA9n+IV8Bm9kiB73bTdBN2R70Jfc3qULC92++vkGQfvPpaLRlEbqT/e4/WP1o11Ku09x9wFaCvxF

Steps to reproduce

Just need to open the link: reproduce link

What is expected?

When the width or height attribute of the emmebbed tag such as <img /> is not a integer type, it will be rewrite to 0.

I have known that mdn specify the height or width of <img /> must be an integer without a unit. But in fact for most browsers, when we set the width or height of <img /> with unit, it can also work.

So when user give a non pure numerical value to the width or height attribute of <img />, vue should preserve the attribute written by the user on the HTML tag instead of rewriting it to 0. The specific rendering behavior is left to the browser to decide.

What is actually happening?

The behavior is different between vue and html:

In vue:
image

In html:
image

System Info

Windows10, Chrome 109

Any additional comments?

No response

@edison1105
Copy link
Member

edison1105 commented Jul 14, 2023

should be width="200" without px.
see https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/width
for more detail see #7658
IMO, this should be fixed because the behavior of CSR and SSR is inconsistent, see SSR.

@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 14, 2023

should be width="200" without px. see https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/width

I know this point, but why not preserve the attribute written by the user, instead, rewrite it to 0? Mdn did not say that when a value has units, it needs to be rewritten to 0.

In standard html, when give dom a invalid attribute or a invalid value of attribute, html will preserve it. So I don't think rewrite it to 0 is a good choice.

@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 14, 2023

In native HTML, the width of can be expressed in units. It is obvious that the content of this part in MDN is inconsistent with the behavior of native HTML. How can you ensure that the content of MDN is always correct?

@edison1105
Copy link
Member

@zh-lx
we should follow the spec

@starcsb
Copy link

starcsb commented Jul 14, 2023

Agree with this issue. Currently, native HTML, react, solid, and svelte can all support width with units, but only Vue does not support it, which is very awkward.

@edison1105 edison1105 added the need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. label Jul 14, 2023
@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 14, 2023

I agree. But when a user gives an invalid value, vue should keep this value and let the HTML handle it according to browser's standard, rather than handling it at the Vue level.

@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 14, 2023

As the most used browser doesn't support this usage, I'd say there's no reason for developers to keep this usage in their application code. This should be fixed during development and not to be shipped to production. Maybe we can warn about this usage at dev time.

Firstly, contrary to what you said, most browsers support this usage, and in fact, I don't know which browser does not support this usage, could you please give me an example?

Another point I want to express is that if a user writes an incorrect HTML fragment in the template section of a Vue, the Vue should keep the corresponding HTML fragment intact and leave it to the browser to process the HTML, rather than modifying it into an HTML fragment that is inconsistent with the user's code.

@Justineo
Copy link
Member

Justineo commented Jul 14, 2023

Firstly, contrary to what you said, most browsers support this usage, and in fact, I don't know which browser does not support this usage.

I deleted my comment as I misremembered it from our last discussion about this in #7658. In most cases Vue tries to set properties if they exist. Shall we keep all invalid attributes in HTML? If so I think we need to always prefer setting attributes for string values. This also solves the inconsistency described at #7658 (comment).

@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 14, 2023

Firstly, contrary to what you said, most browsers support this usage, and in fact, I don't know which browser does not support this usage.

I deleted my comment as I misremembered it from our last discussion about this in #7658. In most cases Vue tries to set properties if they exist. Shall we keep all invalid attributes in HTML? If so I think we need to always prefer setting attributes for string values. This also solves the inconsistency described at #7658 (comment).

I think it is necessary to preserve the attributes in all user code. For example, sometimes users may add an attribute to the dom that is not in the HTML specification and unify the dom with that attribute. Therefore, Vue should preserve the user's code.

@Justineo
Copy link
Member

Justineo commented Jul 14, 2023

For example, sometimes users may add an attribute to the dom that is not in the HTML specification and unify the dom with that attribute.

That’s not the case here. Vue only sets bindings as properties if they exist on the element. Unknown attributes are always set as-is.

@zh-lx
Copy link
Contributor Author

zh-lx commented Jul 17, 2023

For example, sometimes users may add an attribute to the dom that is not in the HTML specification and unify the dom with that attribute.

That’s not the case here. Vue only sets bindings as properties if they exist on the element. Unknown attributes are always set as-is.

Yes, I just made an analogy of this situation. When you give a non integer value to the width of the img tag in a pure HTML file, the HTML will retain your code and let the browser handle it, but Vue will change your code to 0, which I think is not very expected.

@your-brain404
Copy link

I've just encountered that strange behaviour. I wish vuejs/core team member would handle that issue.

@zlx1134558955
Copy link

zlx1134558955 commented Aug 1, 2023

At present, <img width="200px" /> can render normally in SSR mode, but the width will be rewritten to 0 in CSR mode, I think this is a bug. At least the Vue team should ensure that this situation remains consistent in both ssr and csr modes. @edison1105 @Justineo I hope you can pay more attention to this issue. Thanks very much!

@zdm
Copy link

zdm commented Dec 6, 2023

This patch breaks everything.

I am using custom components:

<ext-dialog width="500" height="90%">

No width / height passwd to the component.

@zh-lx
Copy link
Contributor Author

zh-lx commented Dec 7, 2023

This patch breaks everything.

I am using custom components:

<ext-dialog width="500" height="90%">

No width / height passwd to the component.

I don't think it was caused by this change, but was caused by 9845f1d. And I add a new PR to fix this problem: #9763

@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants