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

1.6.x: underscore prefix is not ignored in attribute directive's name #16278

Closed
1 of 3 tasks
curlydevil opened this issue Oct 15, 2017 · 9 comments
Closed
1 of 3 tasks

Comments

@curlydevil
Copy link
Contributor

curlydevil commented Oct 15, 2017

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:
Attribute directive registered as e.g. "t" and used on elements as "_t" is not working.

Expected / new behavior:
Attribute directive registered as e.g. "t" can be used as "_t" again.

Minimal reproduction of the problem with instructions:
comment/uncomment 1.6 vs 1.5 ng version to see the difference
https://plnkr.co/edit/earjy4mhjccYIexn7WYk?p=preview

AngularJS version: 1.x.y
1.5.x - works as described in 'expected' section, 1.6.x - does not work.

Browser:
not a browser-specific issue

Anything else:
My best guess, this bug was introduced in this commit:
73050cd#diff-a732922b631efed1b9f33a24082ae0dbR3600

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Oct 15, 2017

I can confirm that this is a bug. It appears that the regex replace is converting the _t to T rather than t, which stops it from being matched as a directive.

@petebacondarwin
Copy link
Contributor

Previously our camelCase utility didn't upper case the letter if the preceding "special" character was at the start of the name:

   replace(SPECIAL_CHARS_REGEXP, function(_, separator, letter, offset) {
      return offset ? letter.toUpperCase() : letter;
    }).

@petebacondarwin
Copy link
Contributor

But I wonder if this change was "on purpose" as part of 73050cd... @mgol could say.

@petebacondarwin
Copy link
Contributor

One option to fix this could be to update the PREFIX_REGEXP to capture and remove these special character prefixes before they get to the fnCamelCaseReplace if this is functioning as required for camel casing in general.

@curlydevil
Copy link
Contributor Author

Is there any chance for this to be fixed in the nearest future?

@petebacondarwin
Copy link
Contributor

@mgol confirmed that it is a regression. We now need a PR to fix it. Fancy having a go at that @curlydevil ?

@curlydevil
Copy link
Contributor Author

@petebacondarwin I'll try)

@curlydevil
Copy link
Contributor Author

@petebacondarwin I've created a PR but created it into master... should I (in case it passes CR and gets merged) create another one for v1.6.x branch?

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Nov 1, 2017

No against master is fine. We will cherry-pick as necessary.

Narretz pushed a commit that referenced this issue Nov 17, 2017
This fixes regression bug
when directive name with preceeding special char
in HTML markup does not match the registered name.
(introduced in 73050cd)

Closes #16314
Closes #16278
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants