opendev/gerrit
Code Issues Proposed changes

Revert "Revert "Use only Gerrit.install as a plugin loaded signal""

Browse Source 
This reverts commit 96d037968f.

It also contains the underyling fix for loading plugins correctly. The
importHref function expected functions for onload and onerror arguments
and passing null instead was causing the unexplained behavior.

Change-Id: Ic2a36d9bfcf85e5769c26ab49caf15e572d9463e
(cherry picked from commit 7ab0d543e5)
This commit is contained in:
Becky Siegel
2017-10-13 15:36:28 -07:00
committed by Paladox none
parent 8d67ea4d38
commit 7dacf252ec
4 changed files with 55 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
View File

@@ -93,7 +93,8 @@ limitations under the License.
plugin: {
js_resource_paths: [],
html_resource_paths: [
new URL('test/plugin.html', window.location.href).toString(),
new URL('test/plugin.html?' + Math.random(),
window.location.href).toString(),
],
},
};

2
polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html
View File

@@ -24,6 +24,8 @@ limitations under the License.
<link rel="import" href="gr-endpoint-decorator.html">
<link rel="import" href="../gr-endpoint-param/gr-endpoint-param.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<gr-endpoint-decorator name="foo">

6
polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
View File

@@ -47,12 +47,12 @@
* States that it expects no more than 3 parameters, but that's not true.
* @todo (beckysiegel) check Polymer annotations and submit change.
*/
_importHtmlPlugins(plugins) {
for (const url of plugins) {
// onload (second param) needs to be a function. When null or undefined
// were passed, plugins were not loaded correctly.
this.importHref(
this._urlFor(url), Gerrit._pluginInstalled, Gerrit._pluginInstalled,
true);
this._urlFor(url), () => {}, Gerrit._pluginInstalled, true);
}
},

68
polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
View File

@@ -23,6 +23,8 @@ limitations under the License.
<link rel="import" href="../../../test/common-test-setup.html"/>
<link rel="import" href="gr-plugin-host.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<gr-plugin-host></gr-plugin-host>
@@ -60,22 +62,38 @@ limitations under the License.
element.config = {
plugin: {html_resource_paths: ['foo/bar', 'baz']},
};
assert.isTrue(element.importHref.calledWith(
'/foo/bar', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true));
assert.isTrue(element.importHref.calledWith(
'/baz', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true));
assert.equal(element.importHref.firstCall.args[0], '/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.secondCall.args[3]);
});
test('imports relative html plugins from config with a base url', () => {
sandbox.stub(element, 'getBaseUrl').returns('/the-base');
element.config = {
plugin: {html_resource_paths: ['foo/bar', 'baz']}};
assert.isTrue(element.importHref.calledWith(
'/the-base/foo/bar', Gerrit._pluginInstalled, Gerrit._pluginInstalled,
true));
assert.isTrue(element.importHref.calledWith(
'/the-base/baz', Gerrit._pluginInstalled, Gerrit._pluginInstalled,
true));
assert.equal(element.importHref.firstCall.args[0], '/the-base/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/the-base/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.secondCall.args[3]);
});
test('inportHref is not called with null callback functions', () => {
const plugins = ['path/to/plugin'];
element._importHtmlPlugins(plugins);
assert.isTrue(element.importHref.calledOnce);
assert.isFunction(element.importHref.lastCall.args[1]);
assert.isFunction(element.importHref.lastCall.args[2]);
});
test('imports absolute html plugins from config', () => {
@@ -87,12 +105,17 @@ limitations under the License.
],
},
};
assert.isTrue(element.importHref.calledWith(
'http://example.com/foo/bar', Gerrit._pluginInstalled,
Gerrit._pluginInstalled, true));
assert.isTrue(element.importHref.calledWith(
'https://example.com/baz', Gerrit._pluginInstalled,
Gerrit._pluginInstalled, true));
assert.equal(element.importHref.firstCall.args[0],
'http://example.com/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0],
'https://example.com/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.secondCall.args[3]);
});
test('adds js plugins from config to the body', () => {
@@ -138,10 +161,15 @@ limitations under the License.
html_resource_paths: ['some'],
},
};
assert.isTrue(element.importHref.calledWith(
'/oof', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true));
assert.isTrue(element.importHref.calledWith(
'/some', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true));
assert.equal(element.importHref.firstCall.args[0], '/oof');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/some');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.secondCall.args[3]);
});
});
</script>