-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
fix(ivy): match class and attribute value without case-sensitivity #32548
Conversation
Adds two acceptance tests to show a current difference in behavior between Ivy and VE. A directive with a selector `.Titledir` matches an element with `class="titleDir"` in VE but not in Ivy. Same thing for an attribute value.
Prior to this commit, a directive with a selector `selector=".Titledir"` would not match an element like `div class="titleDir"` in Ivy whereas it would in VE. The same issue was present for `selector="[title=Titledir]` and `title="titleDir"`. This fixes the Ivy behavior by changing the matching algorithm to use lowercased values. Note that some `render3` tests needed to be changed to reflect that the compiler generates lowercase selectors. These tests are in the process to be migrated to `acceptance` to use `TestBed` in another PR anyway.
758c99b
to
952ad69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I'm a bit concerned that the packages/core/src/render3/node_selector_matcher.ts
is quite complex and is becoming slightly more complex, but any improvements are a story for a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…32548) Prior to this commit, a directive with a selector `selector=".Titledir"` would not match an element like `div class="titleDir"` in Ivy whereas it would in VE. The same issue was present for `selector="[title=Titledir]` and `title="titleDir"`. This fixes the Ivy behavior by changing the matching algorithm to use lowercased values. Note that some `render3` tests needed to be changed to reflect that the compiler generates lowercase selectors. These tests are in the process to be migrated to `acceptance` to use `TestBed` in another PR anyway. PR Close #32548
Adds two acceptance tests to show a current difference in behavior between Ivy and VE. A directive with a selector `.Titledir` matches an element with `class="titleDir"` in VE but not in Ivy. Same thing for an attribute value. PR Close angular#32548
…ngular#32548) Prior to this commit, a directive with a selector `selector=".Titledir"` would not match an element like `div class="titleDir"` in Ivy whereas it would in VE. The same issue was present for `selector="[title=Titledir]` and `title="titleDir"`. This fixes the Ivy behavior by changing the matching algorithm to use lowercased values. Note that some `render3` tests needed to be changed to reflect that the compiler generates lowercase selectors. These tests are in the process to be migrated to `acceptance` to use `TestBed` in another PR anyway. PR Close angular#32548
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Prior to this commit, a directive with a selector
selector=".Titledir"
would not match an element likediv class="titleDir"
in Ivy whereas it would in VE. The same issue was present forselector="[title=Titledir]
andtitle="titleDir"
.What is the new behavior?
This fixes the Ivy behavior by changing the matching algorithm to use lowercased values.
Does this PR introduce a breaking change?
(well it can if someone was relying on Ivy behaving differently than VE, but I don't think it is the case).
Other information
Note that some
render3
tests needed to be changed to reflect that the compiler generates lowercase selectors. These tests are in the process to be migrated toacceptance
to useTestBed
in another PR anyway.