Commit 325741fb authored by Robert Knight's avatar Robert Knight

Work around Chrome bug causing sidebar to become invisible

Work around a Chrome bug [1] that can cause the sidebar to become
invisible if:

 1. The sidebar app is loaded from a Chrome extension AND
 2. The current tab was opened by clicking a link inside the sidebar
    app in a different tab.

When the issue occurs, the sidebar web app loads and runs normally but
is just not visible on screen. This happens due to an internal issue in
Chrome which can be avoided adding `rel="noopener"` to all "normal" [2]
links in the client that open URLs in a new tab/window.

Doing so enables Chrome to use a separate process for the Hypothesis
client in the new tab in step (2) than the one used for the Hypothesis
client in step (1). This change also prevents potential tab-jacking
attacks in all browsers that support `rel="noopener"`.

Fixes #516

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=753314
[2] ie. Those which do not use JS to handle the link
parent 973bc4da
'use strict'; 'use strict';
var addAnalytics = require('./ga'); var addAnalytics = require('./ga');
var disableOpenerForExternalLinks = require('./util/disable-opener-for-external-links');
var getApiUrl = require('./get-api-url'); var getApiUrl = require('./get-api-url');
var serviceConfig = require('./service-config'); var serviceConfig = require('./service-config');
require('../shared/polyfills'); require('../shared/polyfills');
...@@ -30,6 +31,9 @@ settings.apiUrl = getApiUrl(settings); ...@@ -30,6 +31,9 @@ settings.apiUrl = getApiUrl(settings);
// _before_ Angular is require'd for the first time. // _before_ Angular is require'd for the first time.
document.body.setAttribute('ng-csp', ''); document.body.setAttribute('ng-csp', '');
// Prevent tab-jacking.
disableOpenerForExternalLinks(document.body);
var angular = require('angular'); var angular = require('angular');
// autofill-event relies on the existence of window.angular so // autofill-event relies on the existence of window.angular so
......
'use strict';
/**
* Prevent windows or tabs opened via links under `root` from accessing their
* opening `Window`.
*
* This makes links with `target="blank"` attributes act as if they also had
* the `rel="noopener"` [1] attribute set.
*
* In addition to preventing tab-jacking [2], this also enables multi-process
* browsers to more easily use a new process for instances of Hypothesis in the
* newly-opened tab and works around a bug in Chrome [3]
*
* [1] https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types#noopener
* [2] https://mathiasbynens.github.io/rel-noopener/
* [3] https://bugs.chromium.org/p/chromium/issues/detail?id=753314
*
* @param {Element} root - Root element
*/
function disableOpenerForExternalLinks(root) {
root.addEventListener('click', event => {
if (event.target.tagName === 'A') {
var linkEl = event.target;
if (linkEl.target === '_blank') {
linkEl.rel = 'noopener';
}
}
});
}
module.exports = disableOpenerForExternalLinks;
'use strict';
var disableOpenerForExternalLinks = require('../disable-opener-for-external-links');
describe('sidebar.util.disable-opener-for-external-links', () => {
var containerEl;
var linkEl;
beforeEach(() => {
containerEl = document.createElement('div');
linkEl = document.createElement('a');
containerEl.appendChild(linkEl);
document.body.appendChild(containerEl);
});
afterEach(() => {
containerEl.remove();
});
function clickLink() {
linkEl.dispatchEvent(new Event('click', {
bubbles: true,
cancelable: true,
}));
}
it('disables opener for external links', () => {
linkEl.target = '_blank';
disableOpenerForExternalLinks(containerEl);
clickLink();
assert.equal(linkEl.rel, 'noopener');
});
it('does not disable opener for internal links', () => {
disableOpenerForExternalLinks(containerEl);
clickLink();
assert.equal(linkEl.rel, '');
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment