Amp skimlinks exclude selector

extensions/amp-skimlinks/0.1/amp-skimlinks.js

@@ -160,7 +160,8 @@ export class AmpSkimlinks extends AMP.BaseElement {
    */
   initSkimlinksLinkRewriter_() {
     const options = {
-      linkSelector: this.skimOptions_.linkSelector,
+      linkSelector: this.skimOptions_.includeSelector,
+      excludeSelector: this.skimOptions_.excludeSelector,
     };
 
     const linkRewriter = this.linkRewriterService_.registerLinkRewriter(

extensions/amp-skimlinks/0.1/link-rewriter/link-rewriter-manager.js

@@ -90,13 +90,7 @@ export class LinkRewriterManager {
    * @param {!function(!Array<!HTMLElement>): !./two-steps-response.TwoStepsResponse} resolveUnknownLinks
    *   - Function to determine which anchor should be replaced and by what URL.
    *     Should return an instance of './two-steps-response.TwoStepsResponse'.
-   * @param {?{linkSelector: string}=} options
-   *   - linkSelector is an optional CSS selector to restrict
-   *    which anchors the link rewriter should handle.
-   *    Anchors not matching the CSS selector will be ignored.
-   *    If not provided the link rewrite will handle all the links
-   *    found on the page.
-   *
+   * @param {?./link-rewriter.LinkRewriterOptions=} options
    * @return {!./link-rewriter.LinkRewriter}
    * @public
    */

extensions/amp-skimlinks/0.1/link-rewriter/link-rewriter.js

@@ -19,11 +19,18 @@ import {EVENTS, ORIGINAL_URL_ATTRIBUTE} from './constants';
 import {LinkReplacementCache} from './link-replacement-cache';
 import {Observable} from '../../../../src/observable';
 import {TwoStepsResponse} from './two-steps-response';
+import {nodeListToArray} from '../utils';
 import {userAssert} from '../../../../src/log';
 
 /** @typedef {!Array<{anchor: !HTMLElement, replacementUrl: ?string}>}} */
 export let AnchorReplacementList;
 
+/** @typedef {{
+ * linkSelector: (string|undefined),
+ * excludeSelector: (string|undefined)
+ * }} */
+export let LinkRewriterOptions;
+
 /**
  * LinkRewriter works together with LinkRewriterManager to allow rewriting
  * links at click time. E.g: Replacing a link by its affiliate version only if
@@ -47,7 +54,14 @@ export class LinkRewriter {
    * @param {!Document|!ShadowRoot} rootNode
    * @param {string} id
    * @param {function(!Array<!HTMLElement>):!TwoStepsResponse} resolveUnknownLinks
-   * @param {?{linkSelector: string}=} options
+   * @param {?LinkRewriterOptions=} options
+   *   - "linkSelector" option is an optional CSS selector to restrict
+   *     which anchors the link rewriter should handle.
+   *     Only anchors matching the CSS selector will be considered for affiliate.
+   *     If not provided the link rewrite will handle all the links
+   *     found on the page.
+   *   - "excludeSelector" option is an optional CSS selector. All anchors
+   *     matching this selector will be ignored by amp-skimlinks.
    */
   constructor(rootNode, id, resolveUnknownLinks, options) {
     /** @public {!../../../../src/observable.Observable} */
@@ -65,6 +79,9 @@ export class LinkRewriter {
     /** @private {string} */
     this.linkSelector_ = options.linkSelector || 'a';
 
+    /** @private {string} */
+    this.excludeSelector_ = options.excludeSelector || '';
+
     /** @private {number} */
     this.restoreDelay_ = 300; //ms
 
@@ -188,8 +205,8 @@ export class LinkRewriter {
 
     // Register all new anchors discovered as "unknown" status.
     // Note: Only anchors with a status will be considered in the click
-    // handlers. (Other anchors are assumed to be the ones exluded by
-    // linkSelector_)
+    // handlers. (Other anchors are assumed to be the ones excluded by
+    // linkSelector_ and excludeSelector_)
     this.anchorReplacementCache_.updateReplacementUrls(
       unknownAnchors.map(anchor => ({anchor, replacementUrl: null}))
     );
@@ -236,12 +253,24 @@ export class LinkRewriter {
 
   /**
    * Get the list of anchors element in the page.
-   * (Based on linkSelector option)
+   * (Based on linkSelector & excludeSelector options)
    * @return {!Array<!HTMLElement>}
    * @private
    */
   getLinksInDOM_() {
-    const q = this.rootNode_.querySelectorAll(this.linkSelector_);
-    return [].slice.call(q);
+    const includedNodeList = this.rootNode_.querySelectorAll(
+      this.linkSelector_
+    );
+    // Convert from NodeList to array
+    let anchors = nodeListToArray(includedNodeList);
+
+    if (this.excludeSelector_) {
+      const excludedAnchors = nodeListToArray(
+        this.rootNode_.querySelectorAll(this.excludeSelector_)
+      );
+      anchors = anchors.filter(a => excludedAnchors.indexOf(a) === -1);
+    }
+
+    return anchors;
   }
 }

extensions/amp-skimlinks/0.1/skim-options.js

@@ -46,7 +46,8 @@ export function getAmpSkimlinksOptions(element, docInfo) {
     excludedDomains: getExcludedDomains_(element, getInternalDomains_(docInfo)),
     tracking: getTrackingStatus_(element),
     customTrackingId: getCustomTrackingId_(element),
-    linkSelector: getLinkSelector_(element),
+    includeSelector: getIncludeSelector_(element),
+    excludeSelector: getExcludeSelector_(element),
     waypointBaseUrl: getWaypointBaseUrl(element),
     config: getConfig_(element),
   };
@@ -89,7 +90,6 @@ function getPubCode_(element) {
 }
 
 /**
- *
  * @param {!Element} element
  * @return {boolean}
  */
@@ -121,14 +121,35 @@ function getCustomTrackingId_(element) {
 }
 
 /**
- *
  * @param {!Element} element
  * @return {?string}
  */
-function getLinkSelector_(element) {
-  const linkSelector = element.getAttribute('link-selector');
+function getIncludeSelector_(element) {
+  let includeSelector = element.getAttribute('include-selector');
+  if (!includeSelector) {
+    // 'link-selector' is a deprecated equivalent of 'include-selector'.
+    includeSelector = element.getAttribute('link-selector');
+  }
+
+  return includeSelector || null;
+}
 
-  return linkSelector || null;
+/**
+ *
+ * @param {!Element} element
+ * @return {?string}
+ */
+function getExcludeSelector_(element) {
+  // Always exclude:
+  //  - Links with "noskimlinks" class.
+  //  - Links within a parent container (direct or not) with a "noskimlinks" class.
+  const defaultExcludeSelector = 'a.noskimlinks, .noskimlinks a';
+  const userExcludeSelector = element.getAttribute('exclude-selector');
+  const excludeSelector = userExcludeSelector
+    ? userExcludeSelector.concat(`, ${defaultExcludeSelector}`)
+    : defaultExcludeSelector;
+
+  return excludeSelector;
 }
 
 /**

extensions/amp-skimlinks/0.1/test/test-amp-skimlinks.js

@@ -95,7 +95,7 @@ describes.fakeWin(
           'excluded-domains': 'amazon.com  amazon.fr ',
           tracking: false,
           'custom-tracking-id': 'campaignX',
-          'link-selector': '.content a',
+          'include-selector': '.content a',
         };
         ampSkimlinks = helpers.createAmpSkimlinks(options);
         env.sandbox.stub(ampSkimlinks, 'startSkimcore_');
@@ -107,7 +107,7 @@ describes.fakeWin(
             pubcode: options['publisher-code'],
             tracking: options['tracking'],
             customTrackingId: options['custom-tracking-id'],
-            linkSelector: options['link-selector'],
+            includeSelector: options['include-selector'],
           });
           expect(ampSkimlinks.skimOptions_.excludedDomains).to.include.members([
             'amazon.com',
@@ -121,7 +121,7 @@ describes.fakeWin(
 
         beforeEach(() => {
           ampSkimlinks.skimOptions_ = {
-            linkSelector: '.article a',
+            includeSelector: '.article a',
           };
           ampSkimlinks.linkRewriterService_ = new LinkRewriterManager(
             env.ampdoc
@@ -152,7 +152,7 @@ describes.fakeWin(
           expect(args[0]).to.equal(SKIMLINKS_REWRITER_ID);
           expect(args[1]).to.be.a('function');
           expect(args[2].linkSelector).to.equal(
-            ampSkimlinks.skimOptions_.linkSelector
+            ampSkimlinks.skimOptions_.includeSelector
           );
         });
 

extensions/amp-skimlinks/0.1/test/test-link-rewriter.js

@@ -590,6 +590,52 @@ describes.fakeWin('Link Rewriter', {amp: true}, env => {
 
         expect(resolveFunction.firstCall.args[0]).to.deep.equal([anchor2]);
       });
+
+      it('Should ignore links matching excludeSelector', () => {
+        const anchor1 = rootDocument.createElement('a');
+        const anchor2 = rootDocument.createElement('a');
+        anchor2.classList.add('no-affiliate');
+        const anchor3 = rootDocument.createElement('a');
+
+        rootDocument.body.appendChild(anchor1);
+        rootDocument.body.appendChild(anchor2);
+        rootDocument.body.appendChild(anchor3);
+
+        const resolveFunction = createResolveResponseHelper();
+        createLinkRewriterHelper(resolveFunction, {
+          excludeSelector: 'a.no-affiliate',
+        }).scanLinksOnPage_();
+
+        expect(resolveFunction.firstCall.args[0]).to.deep.equal([
+          anchor1,
+          anchor3,
+        ]);
+      });
+
+      it('excludeSelector should have priority over linkSelector', () => {
+        const anchor1 = rootDocument.createElement('a');
+        anchor1.classList.add('affiliate');
+        const anchor2 = rootDocument.createElement('a');
+        anchor2.classList.add('affiliate');
+        const anchor3 = rootDocument.createElement('a');
+
+        const noskimContainer = rootDocument.createElement('div');
+        noskimContainer.classList.add('no-affiliate');
+
+        rootDocument.body.appendChild(noskimContainer);
+        // Anchor1 has affiliate class but is inside a container with no-affiliate.
+        noskimContainer.appendChild(anchor1);
+        rootDocument.body.appendChild(anchor2);
+        rootDocument.body.appendChild(anchor3);
+
+        const resolveFunction = createResolveResponseHelper();
+        createLinkRewriterHelper(resolveFunction, {
+          linkSelector: 'a.affiliate',
+          excludeSelector: '.no-affiliate a',
+        }).scanLinksOnPage_();
+
+        expect(resolveFunction.firstCall.args[0]).to.deep.equal([anchor2]);
+      });
     });
 
     describe('In dynamic page', () => {

extensions/amp-skimlinks/0.1/test/test-skim-options.js

@@ -141,5 +141,74 @@ describes.fakeWin(
         expect(options.waypointBaseUrl).to.equal(`http://${cname}`);
       });
     });
+
+    describe('include-selector', () => {
+      it('Should be null by default', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.includeSelector).to.be.null;
+      });
+
+      it('Should read the "include-selector" option', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+          'include-selector': 'article a',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.includeSelector).to.equal('article a');
+      });
+
+      it('Should read deprecated "link-selector" option', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+          // legacy equivalent of 'include-selector'
+          'link-selector': 'article a',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.includeSelector).to.equal('article a');
+      });
+
+      it('Should prioritise "include-selector" over "link-selector" option', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+          'include-selector': 'article a',
+          // legacy equivalent of 'include-selector'
+          'link-selector': 'article.press a',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.includeSelector).to.equal('article a');
+      });
+    });
+
+    describe('exclude-selector', () => {
+      it('Should have the noskimlinks exclude selector by default', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.excludeSelector).to.equal(
+          'a.noskimlinks, .noskimlinks a'
+        );
+      });
+
+      it('Should have both the noskimlinks exclude selector and the custom exclude selector when defined', () => {
+        const element = helpers.createAmpSkimlinksElement({
+          'publisher-code': '123X123',
+          'exclude-selector': '.no-affiliate a',
+        });
+        const options = getAmpSkimlinksOptions(element, docInfo);
+
+        expect(options.excludeSelector).to.equal(
+          '.no-affiliate a, a.noskimlinks, .noskimlinks a'
+        );
+      });
+    });
   }
 );

extensions/amp-skimlinks/0.1/test/validator-amp-skimlinks.html

@@ -29,7 +29,9 @@
         layout="nodisplay"
         publisher-code="123X123"
         excluded-domains="samsung.com  asos.com"
-        link-selector="article:not(.no-skimlinks) a"
+        link-selector="article a"
+        include-selector="article a"
+        exclude-selector=".no-affiliate a"
         custom-tracking-id="phones"
         custom-redirect-domain="go.publisher.com"
     ></amp-skimlinks>

extensions/amp-skimlinks/0.1/test/validator-amp-skimlinks.out

@@ -30,7 +30,9 @@ PASS
 |          layout="nodisplay"
 |          publisher-code="123X123"
 |          excluded-domains="samsung.com  asos.com"
-|          link-selector="article:not(.no-skimlinks) a"
+|          link-selector="article a"
+|          include-selector="article a"
+|          exclude-selector=".no-affiliate a"
 |          custom-tracking-id="phones"
 |          custom-redirect-domain="go.publisher.com"
 |      ></amp-skimlinks>

extensions/amp-skimlinks/0.1/utils.js

@@ -78,3 +78,12 @@ export function isExcludedAnchorUrl(anchor, skimOptions) {
   const domain = getNormalizedHostnameFromAnchor(anchor);
   return isExcludedDomain(domain, skimOptions);
 }
+
+/**
+ * Convert a NodeList to an Array.
+ * @param {NodeList} nodeList
+ * @return {!Array<!HTMLElement>}
+ */
+export function nodeListToArray(nodeList) {
+  return [].slice.call(nodeList);
+}

extensions/amp-skimlinks/validator-amp-skimlinks.protoascii

@@ -45,6 +45,12 @@ tags: {  # <amp-skimlinks>
   attrs: {
     name: "link-selector"
   }
+  attrs: {
+    name: "include-selector"
+  }
+  attrs: {
+    name: "exclude-selector"
+  }
   attrs: {
     name: "publisher-code"
     value_regex_casei: "^[0-9]+X[0-9]+$"