From b4cd3067e05b9324a4961f2765bf37d1c98a3b25 Mon Sep 17 00:00:00 2001 From: Logan Hanks Date: Tue, 2 May 2017 09:46:50 -0700 Subject: [PATCH] Disable target="_blank" on MyMenu items Each item of the My menu (called Your in PolyGerrit) is configurable by the user. There is a target property on each item, but currently no UI by which the user can set it. Therefore generally we can expect menu items to receive the default value for this property. Unfortunately, the default value is contingent on whether the server thinks the URL is a view in the GWT UI (by looking for a leading '#'). PolyGerrit URLs do not conform to this expectation, so our default menu items are typically presented by the server as pop-out links. Previously this wasn't a concern because the top menu didn't support pop-out links. We introduced support for this with the Documentation menu in https://gerrit-review.googlesource.com/c/104393/. Bug: Issue 6109 Bug: Issue 8373 Change-Id: I2455bca5ca6250939ff0a9c3b62679aa6d24dadb (cherry picked from commit 26bdf6bab524fde3544a68cecc822e8fa6aa45e3) --- .../elements/core/gr-main-header/gr-main-header.js | 14 ++++++++++++-- .../core/gr-main-header/gr-main-header_test.html | 6 ++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js index affa8e599d..ab042c465a 100644 --- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js +++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js @@ -244,7 +244,7 @@ this.$.restAPI.getPreferences().then(function(prefs) { this._userLinks = - prefs.my.map(this._stripHashPrefix).filter(this._isSupportedLink); + prefs.my.map(this._fixMyMenuItem).filter(this._isSupportedLink); }.bind(this)); this._loadAccountCapabilities(); }, @@ -260,10 +260,20 @@ }.bind(this)); }, - _stripHashPrefix: function(linkObj) { + _fixMyMenuItem: function(linkObj) { + // Normalize all urls to PolyGerrit style. if (linkObj.url.indexOf('#') === 0) { linkObj.url = linkObj.url.slice(1); } + + // Delete target property due to complications of + // https://bugs.chromium.org/p/gerrit/issues/detail?id=5888 + // + // The server tries to guess whether URL is a view within the UI. + // If not, it sets target='_blank' on the menu item. The server + // makes assumptions that work for the GWT UI, but not PolyGerrit, + // so we'll just disable it altogether for now. + delete linkObj.target; return linkObj; }, diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html index 4582b4f9c2..94f1f66f8d 100644 --- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html +++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html @@ -53,14 +53,16 @@ limitations under the License. sandbox.restore(); }); - test('strip hash prefix', function() { + test('fix my menu item', function() { assert.deepEqual([ {url: '#/q/owner:self+is:draft'}, {url: 'https://awesometown.com/#hashyhash'}, - ].map(element._stripHashPrefix), + {url: 'url', target: '_blank'}, + ].map(element._fixMyMenuItem), [ {url: '/q/owner:self+is:draft'}, {url: 'https://awesometown.com/#hashyhash'}, + {url: 'url'}, ]); });