opendev
/
gerrit
Code Issues Proposed changes

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 26bdf6bab5)
This commit is contained in:
Logan Hanks 2017-05-02 09:46:50 -07:00 committed by Paladox none
parent 95cf21e7d3
commit b4cd3067e0
2 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
View File

@ -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;
},

6
polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
View File

@ -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'},
]);
});