From 1758d0297267a8d46042d2f926eebb2263a345e5 Mon Sep 17 00:00:00 2001 From: nirmal bhagwani Date: Wed, 7 Apr 2021 17:01:04 -0700 Subject: [PATCH] feat(compiler): support directive selectors with attributes containing `$` (#41567) This commit adds support for `$` in when selecting attributes. Resolves #41244. test(language-service): Add test to expose bug caused by source file change (#41500) This commit adds a test to expose the bug caused by source file change in between typecheck programs. PR Close #41500 PR Close #41567 --- packages/compiler/src/selector.ts | 60 +++++++++++-- .../compiler/test/selector/selector_spec.ts | 90 +++++++++++++++++++ 2 files changed, 143 insertions(+), 7 deletions(-) diff --git a/packages/compiler/src/selector.ts b/packages/compiler/src/selector.ts index b31f75936c..1460206d8d 100644 --- a/packages/compiler/src/selector.ts +++ b/packages/compiler/src/selector.ts @@ -13,11 +13,11 @@ const _SELECTOR_REGEXP = new RegExp( '(([\\.\\#]?)[-\\w]+)|' + // 2: "tag"; 3: "."/"#"; // "-" should appear first in the regexp below as FF31 parses "[.-\w]" as a range // 4: attribute; 5: attribute_string; 6: attribute_value - '(?:\\[([-.\\w*]+)(?:=([\"\']?)([^\\]\"\']*)\\5)?\\])|' + // "[name]", "[name=value]", - // "[name="value"]", - // "[name='value']" - '(\\))|' + // 7: ")" - '(\\s*,\\s*)', // 8: "," + '(?:\\[([-.\\w*\\\\$]+)(?:=([\"\']?)([^\\]\"\']*)\\5)?\\])|' + // "[name]", "[name=value]", + // "[name="value"]", + // "[name='value']" + '(\\))|' + // 7: ")" + '(\\s*,\\s*)', // 8: "," 'g'); /** @@ -94,8 +94,10 @@ export class CssSelector { } } const attribute = match[SelectorRegexp.ATTRIBUTE]; + if (attribute) { - current.addAttribute(attribute, match[SelectorRegexp.ATTRIBUTE_VALUE]); + current.addAttribute( + current.unescapeAttribute(attribute), match[SelectorRegexp.ATTRIBUTE_VALUE]); } if (match[SelectorRegexp.NOT_END]) { inNot = false; @@ -113,6 +115,50 @@ export class CssSelector { return results; } + /** + * Unescape `\$` sequences from the CSS attribute selector. + * + * This is needed because `$` can have a special meaning in CSS selectors, + * but we might want to match an attribute that contains `$`. + * [MDN web link for more + * info](https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors). + * @param attr the attribute to unescape. + * @returns the unescaped string. + */ + unescapeAttribute(attr: string): string { + let result = ''; + let escaping = false; + for (let i = 0; i < attr.length; i++) { + const char = attr.charAt(i); + if (char === '\\') { + escaping = true; + continue; + } + if (char === '$' && !escaping) { + throw new Error( + `Error in attribute selector "${attr}". ` + + `Unescaped "$" is not supported. Please escape with "\\$".`); + } + escaping = false; + result += char; + } + return result; + } + + /** + * Escape `$` sequences from the CSS attribute selector. + * + * This is needed because `$` can have a special meaning in CSS selectors, + * with this method we are escaping `$` with `\$'. + * [MDN web link for more + * info](https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors). + * @param attr the attribute to escape. + * @returns the escaped string.  + */ + escapeAttribute(attr: string): string { + return attr.replace(/\\/g, '\\\\').replace(/\$/g, '\\$'); + } + isElementSelector(): boolean { return this.hasElementSelector() && this.classNames.length == 0 && this.attrs.length == 0 && this.notSelectors.length === 0; @@ -165,7 +211,7 @@ export class CssSelector { } if (this.attrs) { for (let i = 0; i < this.attrs.length; i += 2) { - const name = this.attrs[i]; + const name = this.escapeAttribute(this.attrs[i]); const value = this.attrs[i + 1]; res += `[${name}${value ? '=' + value : ''}]`; } diff --git a/packages/compiler/test/selector/selector_spec.ts b/packages/compiler/test/selector/selector_spec.ts index 076a19418c..47ece1e724 100644 --- a/packages/compiler/test/selector/selector_spec.ts +++ b/packages/compiler/test/selector/selector_spec.ts @@ -126,6 +126,67 @@ import {el} from '@angular/platform-browser/testing/src/browser_util'; expect(matched).toEqual([s1[0], 1]); }); + it('should support "$" in attribute names', () => { + matcher.addSelectables(s1 = CssSelector.parse('[someAttr\\$]'), 1); + + expect(matcher.match(getSelectorFor({attrs: [['someAttr', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + reset(); + + expect(matcher.match(getSelectorFor({attrs: [['someAttr$', '']]}), selectableCollector)) + .toEqual(true); + expect(matched).toEqual([s1[0], 1]); + reset(); + + matcher.addSelectables(s1 = CssSelector.parse('[some\\$attr]'), 1); + + expect(matcher.match(getSelectorFor({attrs: [['someattr', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + + expect(matcher.match(getSelectorFor({attrs: [['some$attr', '']]}), selectableCollector)) + .toEqual(true); + expect(matched).toEqual([s1[0], 1]); + reset(); + + matcher.addSelectables(s1 = CssSelector.parse('[\\$someAttr]'), 1); + + expect(matcher.match(getSelectorFor({attrs: [['someAttr', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + + expect(matcher.match(getSelectorFor({attrs: [['$someAttr', '']]}), selectableCollector)) + .toEqual(true); + expect(matched).toEqual([s1[0], 1]); + reset(); + + matcher.addSelectables(s1 = CssSelector.parse('[some-\\$Attr]'), 1); + matcher.addSelectables(s2 = CssSelector.parse('[some-\\$Attr][some-\\$-attr]'), 2); + + expect(matcher.match(getSelectorFor({attrs: [['some\\$Attr', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + + expect(matcher.match( + getSelectorFor({attrs: [['some-$-attr', 'someValue'], ['some-$Attr', '']]}), + selectableCollector)) + .toEqual(true); + expect(matched).toEqual([s1[0], 1, s2[0], 2]); + reset(); + + + expect(matcher.match(getSelectorFor({attrs: [['someattr$', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + + expect(matcher.match( + getSelectorFor({attrs: [['some-simple-attr', '']]}), selectableCollector)) + .toEqual(false); + expect(matched).toEqual([]); + reset(); + }); + it('should select by attr name only once if the value is from the DOM', () => { matcher.addSelectables(s1 = CssSelector.parse('[some-decor]'), 1); @@ -307,6 +368,35 @@ import {el} from '@angular/platform-browser/testing/src/browser_util'; expect(cssSelector.toString()).toEqual('sometag'); }); + it('should detect attr names with escaped $', () => { + let cssSelector = CssSelector.parse('[attrname\\$]')[0]; + expect(cssSelector.attrs).toEqual(['attrname$', '']); + expect(cssSelector.toString()).toEqual('[attrname\\$]'); + + cssSelector = CssSelector.parse('[\\$attrname]')[0]; + expect(cssSelector.attrs).toEqual(['$attrname', '']); + expect(cssSelector.toString()).toEqual('[\\$attrname]'); + + cssSelector = CssSelector.parse('[foo\\$bar]')[0]; + expect(cssSelector.attrs).toEqual(['foo$bar', '']); + expect(cssSelector.toString()).toEqual('[foo\\$bar]'); + }); + + it('should error on attr names with unescaped $', () => { + expect(() => CssSelector.parse('[attrname$]')) + .toThrowError( + 'Error in attribute selector "attrname$". Unescaped "$" is not supported. Please escape with "\\$".'); + expect(() => CssSelector.parse('[$attrname]')) + .toThrowError( + 'Error in attribute selector "$attrname". Unescaped "$" is not supported. Please escape with "\\$".'); + expect(() => CssSelector.parse('[foo$bar]')) + .toThrowError( + 'Error in attribute selector "foo$bar". Unescaped "$" is not supported. Please escape with "\\$".'); + expect(() => CssSelector.parse('[foo\\$bar$]')) + .toThrowError( + 'Error in attribute selector "foo\\$bar$". Unescaped "$" is not supported. Please escape with "\\$".'); + }); + it('should detect class names', () => { const cssSelector = CssSelector.parse('.someClass')[0]; expect(cssSelector.classNames).toEqual(['someclass']);