From 4c13c2e13ff76659bbec6bf6704015c7fbbbf6c3 Mon Sep 17 00:00:00 2001 From: Wyatt Allen Date: Wed, 6 Sep 2017 12:12:55 -0700 Subject: [PATCH] Set rel="external" on "Switch Account" links When the switch account URL is configured with the same domain as the app, following the link is captured by the PG catchall route. With this change the link is always marked with rel="external" so that it will be disregarded by page.js. Bug: Issue 7158 Change-Id: I584a5e335935815d70fe4d5205b6c0548a52b340 --- .../gr-account-dropdown/gr-account-dropdown.js | 2 +- .../gr-account-dropdown_test.html | 14 ++++++++++---- .../elements/shared/gr-dropdown/gr-dropdown.js | 8 +++++++- .../shared/gr-dropdown/gr-dropdown_test.html | 15 +++++++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js index c147456e67..e95bd416ca 100644 --- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js +++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js @@ -66,7 +66,7 @@ if (switchAccountUrl) { const replacements = {path}; const url = this._interpolateUrl(switchAccountUrl, replacements); - links.push({name: 'Switch account', url}); + links.push({name: 'Switch account', url, external: true}); } links.push({name: 'Sign out', url: '/logout'}); return links; diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html index 6017cb915b..1183d9c0d6 100644 --- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html +++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.html @@ -83,14 +83,20 @@ limitations under the License. // Unparameterized switch account link. let links = element._getLinks('/switch-account'); assert.equal(links.length, 3); - assert.deepEqual(links[1], - {name: 'Switch account', url: '/switch-account'}); + assert.deepEqual(links[1], { + name: 'Switch account', + url: '/switch-account', + external: true, + }); // Parameterized switch account link. links = element._getLinks('/switch-account${path}', '/c/123'); assert.equal(links.length, 3); - assert.deepEqual(links[1], - {name: 'Switch account', url: '/switch-account/c/123'}); + assert.deepEqual(links[1], { + name: 'Switch account', + url: '/switch-account/c/123', + external: true, + }); }); test('_interpolateUrl', () => { diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js index a66ab2ff27..b145e765e1 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js @@ -14,6 +14,9 @@ (function() { 'use strict'; + const REL_NOOPENER = 'noopener'; + const REL_EXTERNAL = 'external'; + Polymer({ is: 'gr-dropdown', @@ -168,7 +171,10 @@ }, _computeLinkRel(link) { - return link.target ? 'noopener' : null; + // Note: noopener takes precedence over external. + if (link.target) { return REL_NOOPENER; } + if (link.external) { return REL_EXTERNAL; } + return null; }, _handleItemTap(e) { diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html index 8654ac8ea0..ab31f7cb4c 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html @@ -72,10 +72,17 @@ limitations under the License. }); test('link rel', () => { - assert.isNull(element._computeLinkRel({url: '/test'})); - assert.equal( - element._computeLinkRel({url: '/test', target: '_blank'}), - 'noopener'); + let link = {url: '/test'}; + assert.isNull(element._computeLinkRel(link)); + + link = {url: '/test', target: '_blank'}; + assert.equal(element._computeLinkRel(link), 'noopener'); + + link = {url: '/test', external: true}; + assert.equal(element._computeLinkRel(link), 'external'); + + link = {url: '/test', target: '_blank', external: true}; + assert.equal(element._computeLinkRel(link), 'noopener'); }); test('_getClassIfBold', () => {