From 26bdf6bab524fde3544a68cecc822e8fa6aa45e3 Mon Sep 17 00:00:00 2001
From: Logan Hanks <logan@google.com>
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
Change-Id: I2455bca5ca6250939ff0a9c3b62679aa6d24dadb
---
 .../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'},
       ]);
     });