From f53d0fd2d01a323e122eaf27c2c6263f758e35e0 Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Fri, 19 Apr 2019 18:30:54 -0700
Subject: [PATCH] fix(ivy): classes should not mess up matching for bound dir
 attributes (#30002)

Previously, we had a bug where directive matching could fail if the directive
attribute was bound and followed a certain number of classes. This is because
in the matching logic, we were treating classes like normal attributes. We
should instead be skipping classes in the attribute matching logic. Otherwise
classes will match for directives with attribute selectors, and as we are
iterating through them in twos (when they are stored as name-only, not in
name-value pairs), it may throw off directive matching for any bound attributes
that come after. This commit changes the directive matching logic to skip
classes altogether.

PR Close #30002
---
 .../core/src/render3/node_selector_matcher.ts |  8 ++++++
 .../core/test/acceptance/directive_spec.ts    | 26 ++++++++++++++++++-
 .../render3/node_selector_matcher_spec.ts     | 24 +++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts
index 107e2b1917..b291587bc2 100644
--- a/packages/core/src/render3/node_selector_matcher.ts
+++ b/packages/core/src/render3/node_selector_matcher.ts
@@ -205,6 +205,14 @@ function findAttrIndexInNode(
         return i;
       } else if (maybeAttrName === AttributeMarker.Bindings) {
         bindingsMode = true;
+      } else if (maybeAttrName === AttributeMarker.Classes) {
+        let value = attrs[++i];
+        // We should skip classes here because we have a separate mechanism for
+        // matching classes in projection mode.
+        while (typeof value === 'string') {
+          value = attrs[++i];
+        }
+        continue;
       } else if (maybeAttrName === AttributeMarker.Template) {
         // We do not care about Template attributes in this scenario.
         break;
diff --git a/packages/core/test/acceptance/directive_spec.ts b/packages/core/test/acceptance/directive_spec.ts
index 92aa4230b2..6764573e7b 100644
--- a/packages/core/test/acceptance/directive_spec.ts
+++ b/packages/core/test/acceptance/directive_spec.ts
@@ -58,6 +58,30 @@ describe('directives', () => {
       expect(nodesWithDirective.length).toBe(1);
     });
 
+    it('should match a mix of bound directives and classes', () => {
+      TestBed.configureTestingModule({declarations: [TestComponent, TitleDirective]});
+      TestBed.overrideTemplate(TestComponent, `
+        <div class="one two" [id]="someId" [title]="title"></div>
+      `);
+
+      const fixture = TestBed.createComponent(TestComponent);
+      const nodesWithDirective = fixture.debugElement.queryAllNodes(By.directive(TitleDirective));
+
+      expect(nodesWithDirective.length).toBe(1);
+    });
+
+    it('should NOT match classes to directive selectors', () => {
+      TestBed.configureTestingModule({declarations: [TestComponent, TitleDirective]});
+      TestBed.overrideTemplate(TestComponent, `
+        <div class="title" [id]="someId"></div>
+      `);
+
+      const fixture = TestBed.createComponent(TestComponent);
+      const nodesWithDirective = fixture.debugElement.queryAllNodes(By.directive(TitleDirective));
+
+      expect(nodesWithDirective.length).toBe(0);
+    });
+
   });
 
-});
\ No newline at end of file
+});
diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts
index 294f43df17..c186e5069d 100644
--- a/packages/core/test/render3/node_selector_matcher_spec.ts
+++ b/packages/core/test/render3/node_selector_matcher_spec.ts
@@ -201,6 +201,30 @@ describe('css selector matching', () => {
                .toBeFalsy(
                    `Selector '[directive=value]' should not match <span [directive]="exp" [value]="otherExp">`);
          });
+
+      it('should match bound attributes that come after classes', () => {
+        expect(isMatching(
+                   'span',
+                   [
+                     AttributeMarker.Classes, 'my-class', 'other-class', AttributeMarker.Bindings,
+                     'title', 'directive'
+                   ],
+                   ['', 'directive', '']))
+            .toBeTruthy(
+                `Selector '[directive]' should match <span class="my-class other-class" [title]="title" [directive]="exp">`);
+      });
+
+      it('should match NOT match classes when looking for directives', () => {
+        expect(isMatching(
+                   'span',
+                   [
+                     AttributeMarker.Classes, 'directive', 'other-class', AttributeMarker.Bindings,
+                     'title'
+                   ],
+                   ['', 'directive', '']))
+            .toBeFalsy(
+                `Selector '[directive]' should NOT match <span class="directive other-class" [title]="title">`);
+      });
     });
 
     describe('class matching', () => {