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

[Failing Test] {{on}} modifier removes valueless attributes #18480

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kturney
Copy link
Contributor

@kturney kturney commented Oct 11, 2019

Noticed today that using the {{on}} modifier on an element with the required attribute removed the attribute.

e.g.
template: <input type='text' required {{on 'input' this.onInput}} >
resulted in
DOM: <input type='text' >

However, adding a value to the attribute caused it to remain.

e.g.
template: <input type='text' required='true' {{on 'input' this.onInput}} >
resulted in DOM
DOM: <input type='text' required >

While adding a failing test for required, I also added checks for other boolean attributes.
autofocus, required, and disabled are all removed, while readonly is preserved.

I believe the problem is with the {{on}} modifier because
template: <input type='text' required >
results in
DOM: <input type='text' required >.

I looked through the {{on}} modifier source and nothing jumped out at me, so maybe it isn't directly an {{on}} problem though?

Side Note:
I made the disabled element a button to show that it is not only input that is affected.

interestingly, the `readonly` assertion passes, while all the others fail
@kturney kturney changed the title [BUGFIX beta] {{on}} modifier removes boolean attributes [Failing Test] {{on}} modifier removes boolean attributes Oct 11, 2019
@pzuraq
Copy link
Contributor

pzuraq commented Nov 12, 2019

This appears to occur with any modifier except {{action}}, which leads me to believe this is a VM issue.

@chancancode
Copy link
Member

We can fix it with an AST transform until we finish the upgrade, which can also be backported to LTS

@rwjblue
Copy link
Member

rwjblue commented Jan 30, 2020

VM upgrade is finished...

@kturney
Copy link
Contributor Author

kturney commented Jan 30, 2020

seems the same as #18712

@rwjblue
Copy link
Member

rwjblue commented Feb 1, 2020

Can you confirm that this is not about boolean attributes, but is actually about valueless attributes (which are often boolean, 😸)?

@kturney
Copy link
Contributor Author

kturney commented Feb 1, 2020

Would adding a video element with a controls attribute to the test confirm this? I didn't realize there was a distinction.

also add assertion for video element
@kturney kturney changed the title [Failing Test] {{on}} modifier removes boolean attributes [Failing Test] {{on}} modifier removes valueless attributes Feb 1, 2020
@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2020

@kturney ya, any attribute would do, even something like <button data-foo {{on 'click' this.wahtever}}>Click Me!</button>. If that works or not will point towards different types of fixes needed...

@kturney
Copy link
Contributor Author

kturney commented Feb 4, 2020

@rwjblue I added a video element with a controls attribute to the test.

@kategengler
Copy link
Member

Is this still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants