Subscribe on changes!

[runtime-dom]: when <img> width and height are of string type, they will be set to `0`

avatar
Feb 6th 2023

Vue version

3.2.47

Link to minimal reproduction

https://sfc.vuejs.org/#eNp9UMtOwzAQ/JXFl17quA+KIAqV+A9fnHSTGGrHrJ0Wqcq/s24qQCBxsmd3PDOei3gJoTiNKEpRxYZsSBAxjWGvvXVhoAQXIGxhgpYGBwumLrTXvlIzm3kMErpwNAkZAVT9em9dVyk+r/hOSjjbQ+pLWK/ARvADOXME4w/Qo+14sbkPH3lj6ttOyvktC0Gk5lmLPqUQS6XOWAfTvBWvsRioU7YZvIzvoyGUkV8ei8ddbR62q6ZdNbt282Rwi9sinjot5hSstV4xyNZ9YpTNGSt2rNTXV8RSzA1IZwK7DZ47uuRQ+raIWpRwneQZN5Pxd9DYNrnZW06+FTT6ZB0WGJ2saThHJBbWYvlDQ/HwhCQJ/QEJ6T/NX9Q/ull20n4S0yeVPKVf

Steps to reproduce

Look the preview: The height of <img> is 0, but it should be 24px

What is expected?

The height of <img> should be 24px

What is actually happening?

When height and width of some embedded element such as <img><video><source> and <canvas> are string type, they will be rendered as 0. And they are rendered normally when they are number type. This is because of the height and width of these elements must be set as attribute when they are string type, but in patchProp they are directly modified by patchDOMProp instead of patchAttr.

image

System Info

No response

Any additional comments?

No response

avatar
Feb 7th 2023

I'm with edison here. The developer should provide a number as that's what's expected by the HTML spec. We should not start adding code to special handle such cases.

avatar
Feb 7th 2023

Although the description of width on MDN must be a integer, it can also be a string type when used in html.I think the description on mdn is not necessarily accurate, and we should be more consistent with the actual use of html.

Here is an example to prove my point:

The attribute of <input> in mdn also has width and height and their description is consistent with that of <img>. But in fact, the width of input can only be changed through style. Neither setAttribute nor prop can change its width, even if its value is a integer. The following is the example about <input >:

https://sfc.vuejs.org/#eNp9j71Ow0AMx1/FeAlITa4MLFVaiY03YPFSEqdJlfuQ75IOUd4dX4oQAonN/w//ZC/4GkI1T4wHrGMjQ0gQOU3hRG6wwUuCBYQ7WKETb6HQakGOXONdTGDjBY45fyzeeBw9vHsZ24fiiVxt7jgFqUhsw3hOrAqg7p9Py7Itr2ttVG3u4MKUYC6tb3k8EmpOCLehTb2ql/2eUIu1+WbhDu83lvYcqmv0Tr9YMou+gkh4gM3Jnt6eNWGfUogHY2LX5N+vsfJyMTpVMrk0WK442vJD/C2yKJhw94Nh1JxZSmHXsrD8x/xV/cPN2JXciusn32yHkA==

avatar
Feb 7th 2023

it can also be a string type when used in html

Certain browsers may support that, but it's not in the spec:

The attributes, if specified, must have values that are valid non-negative integers.

https://html.spec.whatwg.org/multipage/embedded-content-other.html#attr-dim-height

Vue also supports strings, by the way, as long as they are convertible to integers which the spec expects as values. So if you provide a string containing just a number, free of a unit ('100' instead of '100px'), SSR and CSR work the same and work fine:

https://sfc.vuejs.org/#eNp9UEFOwzAQ/MriSy84TluKIAqV+IcvTrpJDLVj1k6LVOXvrNsKKpA42Tsznh3PSbyGUBwmFJWoY0s2JIiYprDV3rowUoITEHYwQ0ejgwVLF9prX6uLmnU8JHRhbxLyBFAPy611fa34PM93UsLR7tJQwbIEG8GP5MwejN/BgLZnYvUQPjNjmisn5eUtG0Gk9kWLIaUQK6WO2ATTvhdvsRipV7YdvYwfkyGUkV/ui6dNYx7XZduV7aZbPRtc47qIh14LqM4x2GyxLMtFBvL+Id0givfW6vtD4l5cepDOBN45em7qlKPpKxG1qOCMZIz7yfNP3Ni1ud9rWr4VNPlkHRYYnWxoPEYkNtbi/sZDMXhAkoR+h4T0n+cv6R/fbDtrP4v5CxFNpeI=

So this PR would "only" add code to be more compatible with nonstandard browser behavior. That is something we could want, but I'm still not exactly convinced it's worth being another special case to have in the codebase.

@Justineo in what way does this break in certain cases in CSR?

avatar
Feb 7th 2023

it can also be a string type when used in html

Certain browsers may support that, but it's not in the spec:

The attributes, if specified, must have values that are valid non-negative integers.

https://html.spec.whatwg.org/multipage/embedded-content-other.html#attr-dim-height

Vue also supports strings, by the way, as long as they are convertible to integers which the spec expects as values. So if you provide a string containing just a number, free of a unit ('100' instead of '100px'), SSR and CSR work the same and work fine:

https://sfc.vuejs.org/#eNp9UEFOwzAQ/MriSy84TluKIAqV+IcvTrpJDLVj1k6LVOXvrNsKKpA42Tsznh3PSbyGUBwmFJWoY0s2JIiYprDV3rowUoITEHYwQ0ejgwVLF9prX6uLmnU8JHRhbxLyBFAPy611fa34PM93UsLR7tJQwbIEG8GP5MwejN/BgLZnYvUQPjNjmisn5eUtG0Gk9kWLIaUQK6WO2ATTvhdvsRipV7YdvYwfkyGUkV/ui6dNYx7XZduV7aZbPRtc47qIh14LqM4x2GyxLMtFBvL+Id0givfW6vtD4l5cepDOBN45em7qlKPpKxG1qOCMZIz7yfNP3Ni1ud9rWr4VNPlkHRYYnWxoPEYkNtbi/sZDMXhAkoR+h4T0n+cv6R/fbDtrP4v5CxFNpeI=

So this PR would "only" add code to be more compatible with nonstandard browser behavior. That is something we could want, but I'm still not exactly convinced it's worth being another special case to have in the codebase.

@Justineo in what way does this break in certain cases in CSR?

All right, thanks for your answer.

avatar
Feb 8th 2023

it can also be a string type when used in html

Certain browsers may support that, but it's not in the spec:

The attributes, if specified, must have values that are valid non-negative integers.

https://html.spec.whatwg.org/multipage/embedded-content-other.html#attr-dim-height

Vue also supports strings, by the way, as long as they are convertible to integers which the spec expects as values. So if you provide a string containing just a number, free of a unit ('100' instead of '100px'), SSR and CSR work the same and work fine:

https://sfc.vuejs.org/#eNp9UEFOwzAQ/MriSy84TluKIAqV+IcvTrpJDLVj1k6LVOXvrNsKKpA42Tsznh3PSbyGUBwmFJWoY0s2JIiYprDV3rowUoITEHYwQ0ejgwVLF9prX6uLmnU8JHRhbxLyBFAPy611fa34PM93UsLR7tJQwbIEG8GP5MwejN/BgLZnYvUQPjNjmisn5eUtG0Gk9kWLIaUQK6WO2ATTvhdvsRipV7YdvYwfkyGUkV/ui6dNYx7XZduV7aZbPRtc47qIh14LqM4x2GyxLMtFBvL+Id0givfW6vtD4l5cepDOBN45em7qlKPpKxG1qOCMZIz7yfNP3Ni1ud9rWr4VNPlkHRYYnWxoPEYkNtbi/sZDMXhAkoR+h4T0n+cv6R/fbDtrP4v5CxFNpeI=

So this PR would "only" add code to be more compatible with nonstandard browser behavior. That is something we could want, but I'm still not exactly convinced it's worth being another special case to have in the codebase.

@Justineo in what way does this break in certain cases in CSR?

I have the same problem. The description of width of <img> in mdn is that must be an integer, but it is not necessarily related to using setAttribute or prop to update width? The fact is that no matter what the type of this attribute is, all browsers will retain the value given by the user on the dom. However, in Vue, if the user gives width a non-numeric string, width will be set to 0 on the actual dom instead of keeping the value given by the user, which is completely illogical.

avatar
Feb 8th 2023

@LinusBorg

in what way does this break in certain cases in CSR?

https://github.com/vuejs/core/blob/6c6fe2c0cd89ce513503b1f85e0ddb696fd81e54/packages/compiler-dom/src/transforms/stringifyStatic.ts#L83-L86

There's currently a createStaticVNode optimization which stringifies vnodes into HTML so it works similar to SSR. You can see the difference here.

But if we decided not to support non-standard syntax which seems to be supported by all major browsers, current behavior should be fine. (And I just found out that browsers actually support not only ${value}px values, but also all string values starts with a number (eg. 24vue) and extract the number value with logic similar to parseInt.)