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

Polymer.PropertiesMixin fails to create property accessors in IE11 for certain important names like "disabled" #5183

Closed
1 of 6 tasks
bashmish opened this issue Apr 7, 2018 · 8 comments

Comments

@bashmish
Copy link

bashmish commented Apr 7, 2018

Description

Polymer.PropertiesMixin fails to create property accessors in IE11 for certain important names like disabled.

Live Demo

https://github.com/bashmish/bug-ie11-polymer-properties-mixin

Steps to Reproduce

Open tests and see what is failing in IE11 comparing to other browsers.

Expected Results

disabled has corresponding accessors that allow to react to changes.

Actual Results

No accessors are created for disabled.

Browsers Affected

This is not clear to me. I think that it's not only IE11 since the logic seems to be a little bit naive for how accessors are created. But in IE11 it's a way more problematic.

  • Chrome
  • Firefox
  • Edge
  • Safari 9
  • Safari 8
  • IE 11

Versions

  • Polymer: 2.6.0
  • webcomponents: 1.2.0
@TimvdLippe
Copy link
Contributor

This likely clashes because IE11 has a property accessor defined for this property: http://jsbin.com/duzeloj/edit?html,console,output

@sorvell sorvell added the docs label Apr 10, 2018
@sorvell
Copy link
Contributor

sorvell commented Apr 10, 2018

In general creating property accessors for native properties like disabled is (unfortunately) not supported. It looks like we failed specifically to list disabled in the docs related to this here: https://www.polymer-project.org/2.0/docs/devguide/data-binding#native-binding. We'll leave this issue open so we can address the docs.

@TimvdLippe
Copy link
Contributor

This issue was moved to Polymer/old-docs-site#2510

@sorvell
Copy link
Contributor

sorvell commented Apr 10, 2018

On second thought disabled should not be native for normal elements (only for form based elements) so it should generally work.

@sorvell sorvell reopened this Apr 10, 2018
@sorvell
Copy link
Contributor

sorvell commented Apr 10, 2018

Ok, we traced this down. It appears that IE defines disabled on HTMLElement. This is unique (and seems incorrect) and is the root of the issue.

PropertiesChanged.createProperties used by PropertiesMixin to create property accessors does not overwrite existing accessors. We do this to make creating your own accessors straightforward. Note that returning the property from static get properties does 3 things (1) observes it as an attribute, (2) ensures the attribute value gets reflected to the property transformed to the given type, (3) creates a property accessor (if one does not already exist on the element prototype).

However, for backwards compatibility we customize this behavior for Polymer.Element and do overwrite existing accessors. This is why the behavior is different there.

As a workaround, you can manually call MyClass.prototype._createPropertyAccessor('disabled') before defining the element. Here's an example (jsbin won't work in IE but the code should):

http://jsbin.com/kudehimofa/edit?html,console,output

@bashmish
Copy link
Author

bashmish commented Apr 10, 2018

Thanks a lot for detailed explanation. As a temporary workaround I was already thinking about smth like that, but was not sure if it was a safe thing to do.

Do I understand correctly that you don't want to support IE11 disabled in PropertiesMixin? Or if you reopened this, you gonna investigate further? So what's the plan?

@stale
Copy link

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 13, 2020
@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!

@stale stale bot closed this as completed Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants