Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Make directives copy attributes with spaces (Fix for issue 5513) #5597

Closed
wants to merge 2 commits into from

Conversation

siddii
Copy link
Contributor

@siddii siddii commented Jan 2, 2014

PR for issue #5513

@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@siddii
Copy link
Contributor Author

siddii commented Jan 2, 2014

I have signed the CLA before. Here is one of my PR's that was merged to master.

@ghost ghost assigned tbosch Jan 3, 2014
@caitp
Copy link
Contributor

caitp commented Jan 3, 2014

I don't know if this is a desirable change or not --- the issue it solves can be worked around using expression parsing, and this may be the preferable solution, if less performant.

While in many cases leading and trailing whitespace is irrelevant for attributes, I think there are cases where it could have negative effects. We were trimming the values for a reason, after all.

I'm not throwing away the issue, but I'm not totally sure it's a needed change. It's unlikely to be a breaking change, but in a few cases it may be, so the only real advantage I see is a minor performance improvement.

@gsklee
Copy link
Contributor

gsklee commented Jan 5, 2014

@caitp I was skeptic about this being a bug at first but then I actually checked the spec and HTML5 does allow literally anything to be the value of an attribute, so I guess this has a point. The trim should happen inside $parse, but attrs should offer the raw value.

One valid use case might be that a directive that's being used to specify a delimiter of some kind, so it can either be delimiter=" ,", delimiter=" / ", or delimiter=" ".

@siddii siddii added cla: no and removed cla: yes labels Feb 19, 2014
@siddii siddii added cla: no and removed cla: yes labels Mar 19, 2014
@siddii siddii added cla: no and removed cla: yes labels Mar 31, 2014
@mary-poppins
Copy link

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@Narretz Narretz changed the title Fix for issue 5513 Make directives copy attributes with spaces (Fix for issue 5513) Oct 19, 2014
@Narretz Narretz modified the milestones: 1.6.x, Backlog May 23, 2016
Narretz added a commit to Narretz/angular.js that referenced this pull request Jun 8, 2016
Narretz pushed a commit to Narretz/angular.js that referenced this pull request Jun 8, 2016
BREAKING CHANGE:

White-space in attributes is no longer trimmed automatically. This includes leading and trailing
whitespace, and attributes that are purely white-space.

This allows developers to use white-space in their attributes, for example as value for
input[type=radio], as a separator in ngList, or as a value in any custom directive binding.

To migrate, attributes that require trimming must now be trimmed manually.

A common cases where stray white-space can cause problems is when
attribute values are compared, for example in an $observer:

```
$attrs.$observe('myAttr', function(newVal) {
  if (newVal === 'false') ...
});
```

Note that `$parse` trims expressions automatically, so attributes with expressions (e.g. directive
bindings) are unlikely to be affected by stray white-space.

Fixes angular#5513
Fixes angular#14539
Closes angular#5597

ngList test
Narretz added a commit to Narretz/angular.js that referenced this pull request Jun 8, 2016
Narretz pushed a commit to Narretz/angular.js that referenced this pull request Jun 8, 2016
BREAKING CHANGE:

White-space in attributes is no longer trimmed automatically. This includes leading and trailing
whitespace, and attributes that are purely white-space.

This allows developers to use white-space in their attributes, for example as value for
input[type=radio], as a separator in ngList, or as a value in any custom directive binding.

To migrate, attributes that require trimming must now be trimmed manually.

A common cases where stray white-space can cause problems is when
attribute values are compared, for example in an $observer:

```
$attrs.$observe('myAttr', function(newVal) {
  if (newVal === 'false') ...
});
```

Note that `$parse` trims expressions automatically, so attributes with expressions (e.g. directive
bindings) are unlikely to be affected by stray white-space.

Fixes angular#5513
Fixes angular#14539
Closes angular#5597

ngList test
@Narretz Narretz closed this in 97bbf86 Jun 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants