Skip to content

Commit

Permalink
fix(ivy): match class and attribute value without case-sensitivity (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@cexbrayat @matsko
cexbrayat authored and matsko committed Sep 10, 2019
1 parent 8a6e54a commit ded5724
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 27 deletions.
10 changes: 8 additions & 2 deletions packages/core/src/render3/node_selector_matcher.ts
View file
Expand Up @@ -21,7 +21,10 @@ const NG_TEMPLATE_SELECTOR = 'ng-template';

function isCssClassMatching(nodeClassAttrVal: string, cssClassToMatch: string): boolean {
const nodeClassesLen = nodeClassAttrVal.length;
const matchIndex = nodeClassAttrVal !.indexOf(cssClassToMatch);
// we lowercase the class attribute value to be able to match
// selectors without case-sensitivity
// (selectors are already in lowercase when generated)
const matchIndex = nodeClassAttrVal.toLowerCase().indexOf(cssClassToMatch);
const matchEndIdx = matchIndex + cssClassToMatch.length;
if (matchIndex === -1 // no match
|| (matchIndex > 0 && nodeClassAttrVal ![matchIndex - 1] !== ' ') // no space before
Expand Down Expand Up @@ -132,7 +135,10 @@ export function isNodeMatchingSelector(
ngDevMode && assertNotEqual(
nodeAttrs[attrIndexInNode], AttributeMarker.NamespaceURI,
'We do not match directives on namespaced attributes');
nodeAttrValue = nodeAttrs[attrIndexInNode + 1] as string;
// we lowercase the attribute value to be able to match
// selectors without case-sensitivity
// (selectors are already in lowercase when generated)
nodeAttrValue = (nodeAttrs[attrIndexInNode + 1] as string).toLowerCase();
}

const compareAgainstClassName = mode & SelectorFlags.CLASS ? nodeAttrValue : null;
Expand Down
19 changes: 12 additions & 7 deletions packages/core/test/render3/content_spec.ts
View file
Expand Up @@ -913,7 +913,8 @@ describe('content projection', () => {
*/
const Child = createComponent('child', function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
ɵɵprojectionDef(['*', [['span', 'title', 'toFirst']], [['span', 'title', 'toSecond']]]);
// selectors are in lowercase once compiled
ɵɵprojectionDef(['*', [['span', 'title', 'tofirst']], [['span', 'title', 'tosecond']]]);
ɵɵelementStart(0, 'div', ['id', 'first']);
{ ɵɵprojection(1, 1); }
ɵɵelementEnd();
Expand Down Expand Up @@ -957,9 +958,10 @@ describe('content projection', () => {
*/
const Child = createComponent('child', function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
// selectors are in lowercase once compiled
ɵɵprojectionDef([
'*', [['span', SelectorFlags.CLASS, 'toFirst']],
[['span', SelectorFlags.CLASS, 'toSecond']]
'*', [['span', SelectorFlags.CLASS, 'tofirst']],
[['span', SelectorFlags.CLASS, 'tosecond']]
]);
ɵɵelementStart(0, 'div', ['id', 'first']);
{ ɵɵprojection(1, 1); }
Expand Down Expand Up @@ -1004,9 +1006,10 @@ describe('content projection', () => {
*/
const Child = createComponent('child', function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
// selectors are in lowercase once compiled
ɵɵprojectionDef([
'*', [['span', SelectorFlags.CLASS, 'toFirst']],
[['span', SelectorFlags.CLASS, 'toSecond']]
'*', [['span', SelectorFlags.CLASS, 'tofirst']],
[['span', SelectorFlags.CLASS, 'tosecond']]
]);
ɵɵelementStart(0, 'div', ['id', 'first']);
{ ɵɵprojection(1, 1); }
Expand Down Expand Up @@ -1095,7 +1098,8 @@ describe('content projection', () => {
*/
const Child = createComponent('child', function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
ɵɵprojectionDef(['*', [['span', SelectorFlags.CLASS, 'toFirst']]]);
// selectors are in lowercase once compiled
ɵɵprojectionDef(['*', [['span', SelectorFlags.CLASS, 'tofirst']]]);
ɵɵelementStart(0, 'div', ['id', 'first']);
{ ɵɵprojection(1, 1); }
ɵɵelementEnd();
Expand Down Expand Up @@ -1140,7 +1144,8 @@ describe('content projection', () => {
*/
const Child = createComponent('child', function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
ɵɵprojectionDef(['*', [['span', SelectorFlags.CLASS, 'toSecond']]]);
// selectors are in lowercase once compiled
ɵɵprojectionDef(['*', [['span', SelectorFlags.CLASS, 'tosecond']]]);
ɵɵelementStart(0, 'div', ['id', 'first']);
{ ɵɵprojection(1); }
ɵɵelementEnd();
Expand Down
34 changes: 16 additions & 18 deletions packages/core/test/render3/node_selector_matcher_spec.ts
View file
Expand Up @@ -94,14 +94,16 @@ describe('css selector matching', () => {
])).toBeTruthy(`Selector '[title]' should match <span id="my_id" title="test_title">`);
});


/**
* We assume that compiler will lower-case all selectors when generating code
*/
it('should match single attribute with value', () => {
expect(isMatching('span', ['title', 'My Title'], [
'', 'title', 'My Title'
'', 'title', 'my title'
])).toBeTruthy(`Selector '[title="My Title"]' should match <span title="My Title">'`);

expect(isMatching('span', ['title', 'My Title'], [
'', 'title', 'Other Title'
'', 'title', 'other title'
])).toBeFalsy(`Selector '[title="Other Title"]' should NOT match <span title="My Title">`);
});

Expand All @@ -111,7 +113,7 @@ describe('css selector matching', () => {
])).toBeFalsy(`Selector 'div[title]' should NOT match <span title="My Title">`);

expect(isMatching('span', ['title', 'My Title'], [
'div', 'title', 'My Title'
'div', 'title', 'my title'
])).toBeFalsy(`Selector 'div[title="My Title"]' should NOT match <span title="My Title">`);
});

Expand Down Expand Up @@ -147,7 +149,8 @@ describe('css selector matching', () => {
});

/**
* We assume that compiler will lower-case all attribute names when generating code
* We assume that compiler will lower-case all selectors and attribute names when generating
* code
*/
it('should match attribute name case-sensitively', () => {
expect(isMatching('span', ['foo', ''], [
Expand All @@ -159,14 +162,10 @@ describe('css selector matching', () => {
])).toBeFalsy(`Selector '[Foo]' should NOT match <span foo>`);
});

it('should match attribute values case-sensitively', () => {
it('should match attribute values case-insensitively', () => {
expect(isMatching('span', ['foo', 'Bar'], [
'', 'foo', 'Bar'
])).toBeTruthy(`Selector '[foo="Bar"]' should match <span foo="Bar">`);

expect(isMatching('span', ['foo', 'Bar'], [
'', 'Foo', 'bar'
])).toBeFalsy(`Selector '[Foo="bar"]' should match <span foo="Bar">`);
'', 'foo', 'bar'
])).toBeTruthy(`Selector '[foo="bar"]' should match <span foo="Bar">`);
});

it('should match class as an attribute', () => {
Expand Down Expand Up @@ -288,14 +287,13 @@ describe('css selector matching', () => {
])).toBeTruthy(`Selector '.bar.foo' should match <span class="bar foo">`);
});

it('should match class name case-sensitively', () => {
expect(isMatching('span', ['class', 'Foo'], [
'', SelectorFlags.CLASS, 'Foo'
])).toBeTruthy(`Selector '.Foo' should match <span class="Foo">`);

/**
* We assume that compiler will lower-case all selectors when generating code
*/
it('should match class name case-insensitively', () => {
expect(isMatching('span', ['class', 'Foo'], [
'', SelectorFlags.CLASS, 'foo'
])).toBeFalsy(`Selector '.foo' should NOT match <span class-"Foo">`);
])).toBeTruthy(`Selector '.Foo' should match <span class="Foo">`);
});

it('should work without a class attribute', () => {
Expand Down

0 comments on commit ded5724

Please sign in to comment.