🐛 Don't append linker for exact domain match, even if in domains. (#…

…32100)

* Don't append linker for exact domain match, even if in `domains`.

* Add linker test with matching window host.

* Update linker docs.

* Linter fixes.

* Adjust tests.

* Improve host name mocking for test.

* Fix new test.

* Added a missing return type to isDomainMatch_ (see #23582).

* Clarify documentation and correct a couple of typos.

extensions/amp-analytics/0.1/linker-manager.js

@@ -301,10 +301,17 @@ export class LinkerManager {
    * @param {Location} location
    * @param {string} name Name given in linker config.
    * @param {?Array} domains
-   * @return {*} TODO(#23582): Specify return type
+   * @return {boolean}
    */
   isDomainMatch_(location, name, domains) {
     const {hostname} = location;
+    // Don't append linker for exact domain match, relative urls, or
+    // fragments.
+    const winHostname = WindowInterface.getHostname(this.ampdoc_.win);
+    if (winHostname === hostname) {
+      return false;
+    }
+
     // If given domains, but not in the right format.
     if (domains && !Array.isArray(domains)) {
       user().warn(TAG, '%s destinationDomains must be an array.', name);
@@ -317,14 +324,6 @@ export class LinkerManager {
     }
 
     // Fallback to default behavior
-
-    // Don't append linker for exact domain match, relative urls, or
-    // fragments.
-    const winHostname = WindowInterface.getHostname(this.ampdoc_.win);
-    if (winHostname === hostname) {
-      return false;
-    }
-
     const {sourceUrl, canonicalUrl} = Services.documentInfoForDoc(this.ampdoc_);
     const canonicalOrigin = this.urlService_.parse(canonicalUrl).hostname;
     const isFriendlyCanonicalOrigin = areFriendlyDomains(

extensions/amp-analytics/0.1/test/test-linker-manager.js

@@ -318,13 +318,15 @@ describes.realWin('Linker Manager', {amp: true}, (env) => {
             ids: {
               id: '222',
             },
-            destinationDomains: ['foo.com', 'bar.com'],
+            destinationDomains: ['foo.com', 'bar.com', 'testdomain.com'],
           },
         },
       };
 
       const lm = new LinkerManager(ampdoc, config, /* type */ null, element);
       return lm.init().then(() => {
+        windowInterface.getHostname.returns('testdomain.com');
+
         // testLinker1 should apply to both canonical and source
         // testLinker2 should not
         const canonicalDomainUrl = clickAnchor(
@@ -346,6 +348,12 @@ describes.realWin('Linker Manager', {amp: true}, (env) => {
         const barDomainUrl = clickAnchor('https://bar.com/path');
         expect(barDomainUrl).to.not.contain('testLinker1=');
         expect(barDomainUrl).to.contain('testLinker2=');
+
+        // When the window host name matches the target,
+        // the linker should not be applied.
+        const localDomainUrl = clickAnchor('https://testdomain.com/path');
+        expect(localDomainUrl).to.not.contain('testLinker1=');
+        expect(localDomainUrl).to.not.contain('testLinker2=');
       });
     });
 
@@ -626,7 +634,7 @@ describes.realWin('Linker Manager', {amp: true}, (env) => {
       });
     });
 
-    it('should add linker if same domain is in destination domains', () => {
+    it('should not add linker if same domain is in destination domains', () => {
       const config = {
         linkers: {
           testLinker: {
@@ -642,7 +650,7 @@ describes.realWin('Linker Manager', {amp: true}, (env) => {
       const lm = new LinkerManager(ampdoc, config, /* type */ null, element);
       return lm.init().then(() => {
         const url = clickAnchor('https://amp.source.test/');
-        expect(url).to.contain('testLinker');
+        expect(url).not.to.contain('testLinker');
       });
     });
 

extensions/amp-analytics/linker-id-forwarding.md

@@ -22,8 +22,8 @@ This document outlines the configuration options that will determine in which co
 -   `paramName` - This user defined name determines the name of the query parameter appended to the links.
 -   `ids` - An object containing key-value pairs that is partially encoded and passed along in the param.
 -   `proxyOnly` - (optional) Flag indicating whether the links should only be appended on pages served on a proxy origin. Defaults to `true`.
--   `destinationDomains` - (optional) Links will be decorated if their domains are included in this array. Defaults to [`canonical`](https://github.com/ampproject/amphtml/blob/3b0feadab3b9b12ddb80edc9a30f959087134905/spec/amp-html-format.md#canon) and `source` domains. A link matching the exact same hostname will not be decorated unless specified in this array.
--   `enabled` - Publishers must explicity set this to `true` to opt-in to using this feature.
+-   `destinationDomains` - (optional) Links will be decorated if their domains are included in this array. Defaults to [`canonical`](https://github.com/ampproject/amphtml/blob/3b0feadab3b9b12ddb80edc9a30f959087134905/spec/amp-html-format.md#canon) and `source` domains. A link matching the exact same hostname will not be decorated unless specified in this array. Links will never be decorated when the window host matches the link host.
+-   `enabled` - Publishers must explicitly set this to `true` to opt-in to using this feature.
 
 This linker uses this configuration to generate a string in this structure: `<paramName>=<version>*<checkSum>*<idName1>*<idValue1>*<idName2>*<idValue2>...` For more details see [Linker Param Format](./linker-id-receiving.md#Format)
 
@@ -61,7 +61,7 @@ An example of a config that grants more granular control may look like the examp
 }
 ```
 
-In this example configuration, the parameter would be appendend to any outgoing links matching the `source` or `canonical` domains that are not an exact hostname match. This is because the `destinationDomains` entry has been omitted and this is the default behavior. The example has `proxyOnly` set to `false`, this overrides the default behavior and indicates that the linker should manage outgoing links in all contexts this amp page might be served in. Finally, we have set `enabled` to be `true`. This is necessary to tell the runtime that we would like to enable this linker configuration.
+In this example configuration, the parameter would be appended to any outgoing links matching the `source` or `canonical` domains that are not an exact hostname match. This is because the `destinationDomains` entry has been omitted and this is the default behavior. The example has `proxyOnly` set to `false`, this overrides the default behavior and indicates that the linker should manage outgoing links in all contexts this amp page might be served in. Finally, we have set `enabled` to be `true`. This is necessary to tell the runtime that we would like to enable this linker configuration.
 
 #### Destination Domain Matching
 
@@ -88,7 +88,7 @@ For example: you may have a site `example.com` that links to `a.example.com`, `b
 "destinationDomains": ["*.example.com"]
 ```
 
-Using this configuration links to `a.example.com`, `b.example.com` and `a.b.example.com` will all be decorated. Similarily, you may want to decorate links to `example.co`, `example.co.uk`, and `example.fr`. This can be accomplished using the config below.
+Using this configuration links to `a.example.com`, `b.example.com` and `a.b.example.com` will all be decorated. Similarly, you may want to decorate links to `example.co`, `example.co.uk`, and `example.fr`. This can be accomplished using the config below.
 
 ```json
 "destinationDomains": ["example.*"]