Angular.js: Undesirable ng-attr-disabled behavior on custom elements

[morewry]

I'm submitting a ...

• [x] bug report
• [ ] feature request
• [ ] other

Current behavior:

ng-attr conditionally sets attributes. For most attributes, including ad hoc custom attributes, this means that if the expression interpolates to a string, that attribute will be set with that string as its value. But certain attributes, such as disabled, are recognized specifically and have different behavior. On some elements (e.g. button, input, etc.) disabled is a boolean HTML attribute and valid HTML requires it be either <button disabled> or <button disabled="disabled">. And that's how ng-attr-disabled behaves. But disabled isn't a global attribute and these semantics and rules really only apply for the standard HTML elements that are specified with the standard disabled attribute.

I have defined a Custom Element that observes a disabled attribute. But due to this behavior, consumers of my Custom Element can't use ng-attr on that custom element for the disabled attribute, because instead of outputting the value they're trying to set, the string disabled always gets output instead.

Actually, it's "worse" than I originally thought. There's no way to set any other value with Angular. See final section.

Expected / new behavior:

React has some similar issues affecting support for Web Components where it attempts to provide value to developers by throwing errors when standard HTML elements and attributes are used in invalid ways in JSX. My loose understanding is that React now checks for a hyphen, assumes any tag with hyphen in its name is a custom element, and goes down a different path without its potentially disruptive HTML validation behavior for those.

I think it might be preferable if ngAttr and ngDisabled (among, probably, some other directives) did something similar. The enforcement of disabled="disabled" should really only apply on standard HTML elements, not on Custom Elements.

Especially since, without the draconian error handling of serving XHTML as XML (perhaps even with it, I don't recall, it's been so long), it really doesn't matter what the value is for any element. Strictly speaking, it's invalid, but practically speaking, you can write the HTML <button disabled="woooooooo!!!"> and it's handled the same in browsers as <button disabled="disabled">.

Minimal reproduction of the problem with instructions:

https://codepen.io/morewry/pen/QxqrmJ

AngularJS version: 1.6.7, 1.7.3

Browser: all

Anything else:

I get that this could be problematic since it's a change in how Angular has behaved for a long time. It's a pain for using Web Components with Angular as-is, however, because it's almost impossible to work around without changing the attribute name, since the behavior shows up in all of the following:

1. When using ng-attr-disabled
2. When using ng-disabled
3. When using disabled with interpolation, e.g. disabled="{{something}}"

But there's no particular reason this attribute name should have to change. The best vocabulary choice is disabled, not disable, not disabled-whatever or whatever-disabled, in part _because_ it's familiar.

[gkalpak]

A little background:

• Special handling of boolean attributes in $compile was introduced long ago in 4f78fd692.
• Later, in de9464c14, boolean attributes where limited to specific elements (as the previous approach was too aggressive).
• However, while boolean attributes are limited to specific elements as far as $compile's $attrs is concerned (e.g. see here), jqLite's attr —which is used by $attrs under the hood to set the attribute value— only takes the attribute name (not the element) into account.
• jqLite#attr() matches the behavior of jQuery#attr().

So, I would say it is reasonable to take the element into account in attr(), but we need to keep it compatible with jQuery 😞

Thoughts @mgol, @jbedard?

_{It is possible to special-case $attrs.$set() to handle boolean properties more consistently, but I would rather avoid special casing if possible.}

[gkalpak]

Discussed this and agreed this is an issue with jQuery. We decided it doesn't make sense to break compatiblity with jQuery atm. (If this is changed in jQuery in the future, we can reconsider.)

In the meantime, it is fairly straight forward to work around this by creating your own directive that set disabled via setAttribute() and not .attr().

[morewry]

Didn't think of that workaround, thanks! It necessitates more education and a habit change for consuming developers, but it'll certainly mitigate.

Do you think I should file this for jQuery? Do you happen to know if the maintainers/community are concerned about Web Component compatibility?

[gkalpak]

No idea about jQuery, but @mgol said he will bring it up in an upcoming meeting.
Probably doesn't hurt to open an issue on their tracker (if one doesn't exist already) :wink:

[mgol]

A relevant jQuery ticket has been opened: https://github.com/jquery/jquery/issues/4114.