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

[BUGFIX beta] Fix range element reporting wrong initial value #425

Closed
wants to merge 1 commit into from

Conversation

Serabe
Copy link
Member

@Serabe Serabe commented Mar 15, 2017

If a template had an input range whose type was set before a max or a min
attribute was added, the initial value would be wrong.

In plain HTML, the initial value for <input max="10" type="range" /> would be
5 ((min + max) / 2 = (0 + 10) / 2 = 5).

In Glimmer, this would be 10 because the code would be equivalent to:

let input = document.createElement('input');
input.type = "range";
input.max = "10";

Setting type to range would make the value sanitization algorihtm to kick
in, setting the value to 50. When setting the max to 10, the algorithm
would sanitize the value (50 by now) to the maximum (10).

More info in whatwg/html#2427
Fixes emberjs/ember.js#14958

If a template had an `input` `range` whose type was set before a max or a min
attribute was added, the initial value would be wrong.

In plain HTML, the initial value for `<input max="10" type="range" />` would be
`5` (`(min + max) / 2 = (0 + 10) / 2 = 5`).

In Glimmer, this would be 10 because the code would be equivalent to:

```js
let input = document.createElement('input');
input.type = "range";
input.max = "10";
```

Setting `type` to `range` would make the value sanitization algorihtm to kick
in, setting the value to 50. When setting the `max` to `10`, the algorithm
would sanitize the value (50 by now) to the maximum (10).

More info in whatwg/html#2427
Fixes emberjs/ember.js#14958
@Serabe
Copy link
Member Author

Serabe commented Mar 15, 2017

Waiting for master to be stable to rebase.

@chancancode
Copy link
Contributor

We can't fix it at this layer because the attribute could come from a few e.g. attributeBindings in the Ember.Component API, so we probably need to fix it at runtime. Setting attributes go through the ElementOperation object which would be the right place to do it.

@Serabe
Copy link
Member Author

Serabe commented Mar 15, 2017

Thank you for the hint. I'll do it tomorrow (kinda late here).

@Turbo87 Turbo87 added the bug label Oct 22, 2017
Serabe added a commit to Serabe/glimmer-vm that referenced this pull request Oct 27, 2017
If a template had an `input` `range` whose type was set before a max or a min
attribute was added, the initial value would be wrong.

In plain HTML, the initial value for `<input max="10" type="range" />` would be
`5` (`(min + max) / 2 = (0 + 10) / 2 = 5`).

In Glimmer, this would be 10 because the code would be equivalent to:

```js
let input = document.createElement('input');
input.type = "range";
input.max = "10";
```

Setting `type` to `range` would make the value sanitization algorihtm to kick
in, setting the value to 50. When setting the `max` to `10`, the algorithm
would sanitize the value (50 by now) to the maximum (10).

This was originally discussed in glimmerjs#425, but an alternative solution was
asked for. While this PR keeps part of the original implementation
(could not find a simple way to fix the issue when a template is being
rendered), this PR goes further and fixes it for Emberish components and
`...attributes` in Glimmer components.

More info in whatwg/html#2427
Fixes emberjs/ember.js#14958
@Serabe
Copy link
Member Author

Serabe commented Oct 27, 2017

Closed in favor of #726

@Serabe Serabe closed this Oct 27, 2017
Serabe added a commit to Serabe/glimmer-vm that referenced this pull request Nov 17, 2017
If a template had an `input` `range` whose type was set before a max or a min
attribute was added, the initial value would be wrong.

In plain HTML, the initial value for `<input max="10" type="range" />` would be
`5` (`(min + max) / 2 = (0 + 10) / 2 = 5`).

In Glimmer, this would be 10 because the code would be equivalent to:

```js
let input = document.createElement('input');
input.type = "range";
input.max = "10";
```

Setting `type` to `range` would make the value sanitization algorihtm to kick
in, setting the value to 50. When setting the `max` to `10`, the algorithm
would sanitize the value (50 by now) to the maximum (10).

This was originally discussed in glimmerjs#425, but an alternative solution was
asked for. While this PR keeps part of the original implementation
(could not find a simple way to fix the issue when a template is being
rendered), this PR goes further and fixes it for Emberish components and
`...attributes` in Glimmer components.

More info in whatwg/html#2427
Fixes emberjs/ember.js#14958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants