Angular.js: new camelCase feature for ngAttr broken by jquery

[jaawerth]

Hi!

I noticed this today: the new functionality for ngAttr allowing for camelCased attributes appears to be overridden when jquery is included on the project.

For example, $scope.foo = '0 0 24 24', ng-attr-view_box="{{foo}}" without jquery becomes viewBox="0 0 24 24 as expected, but when jquery is added to the code, it becomes viewbox="0 0 24 24".

Working plnkr (no jquery)
Broken plnkr (with jquery)

I'll try and follow up with a PR if I can make time to dig into the code, but figured I'd get this reported in the meantime. Thanks!

[neberej]

This is a bug (http://bugs.jquery.com/ticket/11166) on jQuery. jQuery converts characters in attributes to lowercase. SVG may be the only exception where attribute names are not lowercase.

from jquery.js

if ( nType !== 1 || !jQuery.isXMLDoc( elem ) ) {
    name = name.toLowerCase(); <------- this line converts to lowercase
    hooks = jQuery.attrHooks[ name ] ||
    ( jQuery.expr.match.bool.test( name ) ? boolHook : nodeHook );
}

[wesleycho]

jQuery resolved this as wontfix - would a conditional in $compile to check for jQuery's presence when using this form of ngAttr suffice to address this?

[jaawerth]

Just exploring it a bit, I was able to get it working as expected with a quick hack.

Since jQuery replaces jQLite when detected barring a few methods, I preserved .attr (aliased to caseAttr during the jq patch) and added a check for jQuery.fn.caseAttr in compile.js, so $set will use it when needed. I haven't spent a ton of time with the $compile code, but I don't _think_ it will affect any other attributes, since Angular already lowercases all but those that use ng-attr-snake_case.

Anyway, plnkr here (noted the three modified lines in the compiled hacked-angular.js to the description)

1. in src/Angular.js: 1478, added caseAttr: JQLitePrototype.attr

javascript if (jQuery && jQuery.fn.on) { jqLite = jQuery; extend(jQuery.fn, { scope: JQLitePrototype.scope, isolateScope: JQLitePrototype.isolateScope, controller: JQLitePrototype.controller, injector: JQLitePrototype.injector, inheritedData: JQLitePrototype.inheritedData, caseAttr: JQLitePrototype.attr // new line: 1478 });

1. in src/ng/compile.js:893, var attrFn = (jQuery.fn.caseAttr) ? 'caseAttr' : 'attr';

``` javascript
this.$get = [
'$injector', '$interpolate', '$exceptionHandler', '$templateRequest', '$parse',
'$controller', '$rootScope', '$document', '$sce', '$animate', '$$sanitizeUri',
function($injector, $interpolate, $exceptionHandler, $templateRequest, $parse,
$controller, $rootScope, $document, $sce, $animate, $$sanitizeUri) {

   var attrFn = (jQuery.fn.caseAttr) ? 'caseAttr' : 'attr'; // added here, ln 893

   var Attributes = ...

```

1. in src/ng/compile.js:1074, changed to this.$$element.attr(attrName, value) to this.$$element[attrFn](attrName, value);:

javascript if (writeAttr !== false) { if (value === null || value === undefined) { this.$$element.removeAttr(attrName); } else { this.$$element[attrFn](attrName, value); // previously this.$$element.attr(attrName, value) } }

[lgalfaso]

I do not like where this is moving to. I feel that angular should not hack around jQuery (at least not as part of the core). This is, the real patch should be on jQuery

[neberej]

+1

But it seems jQuery wont be fixing this anytime soon either.

http://bugs.jquery.com/ticket/7584

This ticket is being changed to wontfix since it isn't our intent to support SVG in any first-class way. The wontfix page will be updated shortly.
jQuery is an HTML DOM library and despite the fact that SVG uses a similar XML syntax it has different semantics. If you want to manipulate SVG a library like Raphael is more appropriate.

[lgalfaso]

@nabaraz that does not mean that you cannot create a jquery extension that does _the right thing_

Anyhow, if people think that this should be fixed on angular side, then this will need to be in an external module that decorates _stuff_... even when I am not sure how easy/hard that would be

[jaawerth]

I see what you mean, but if it's an external module, what of the current snake_case capability? If jquery is overriding it, it'd be problematic to address the issue with an external module while a core feature will continue not to work as expected with jquery, no?

Or are you saying the solution is to move the ng-attr-snake_case capability into this decorator module as well? "angular-svg" or something akin to that?

[wesleycho]

Not to put words in @lgalfaso 's mouth, but I think the suggestion is to put any jQuery related patching into a separate module. If that is the case then, would all jQuery testing with angular also be changed to require this module? This would have some non-trivial setup complexity on the Angular team's side.

[lgalfaso]

I do not have a specific implementation in mind (I am sure there are several ways to work around the jQuery limitation without making any change to the core using an external module/library). The one thing that I am sure about is that angular should not hack around jQuery.

I am going to keep this issue open a few more days to get some feedback

[caitp]

i think we should actually hack around jQuery here. this isn't a jQuery feature, it's an angular feature. the fact that jQuery breaks it just shows that our current implementation of the feature is wrong/broken.

[travissimon]

@caitp I've just encountered this 'in the wild' nearly a year later. Did you ever manage to look into this? I'm guessing by the elapsed time that I should work around this issue?

[kschaefe]

I changed my approach to add a $watch for my svg viewBox size and manually set the value using
element[0].setAttribute('viewBox', newValue)

Not a fan of having to resort to the low-level JavaScript DOM bindings. What's the timeframe on this?

[timmywil]

This seems to work now with jQuery 3.0: http://plnkr.co/edit/n7dKDitTeg8SgJQznCqs?p=preview

[mgol]

Yup, jQuery 3.0.0 has stopped lowercasing the attribute (due to no longer having to workaround old IE or keeping compatibility with jQuery 1.x). The ticket for that is https://github.com/jquery/jquery/issues/2914. I'm closing this issue then.