mirror of
https://github.com/TryGhost/Ghost-Admin.git
synced 2023-12-14 02:33:04 +01:00
🐛 Fixed Code Injection input fields not being clickable
no issue - lazy loaded scripts such as the CodeMirror asset used on the Code Injection screen could throw errors such as `TypeError: Cannot set property 'modeOption' of undefined` - this was caused by "loading" promise returned from the `lazyLoader` service returning as soon as the network request finished which can be before the loaded script has been parsed and run meaning any processing occurring after the promise returns could be depending on unloaded code - switched the lazyLoader service's loading mechanism from an ajax fetch to insertion of a `<script>` tag which can have `load` event attached which _will_ return after parsing/loading has completed
This commit is contained in:
parent
437e60c347
commit
fefc8358cb
3 changed files with 43 additions and 33 deletions
|
@ -1,6 +1,5 @@
|
|||
/* global CodeMirror */
|
||||
import Component from '@ember/component';
|
||||
import RSVP from 'rsvp';
|
||||
import boundOneWay from 'ghost-admin/utils/bound-one-way';
|
||||
import {assign} from '@ember/polyfills';
|
||||
import {bind, once, scheduleOnce} from '@ember/runloop';
|
||||
|
@ -63,10 +62,7 @@ const CmEditorComponent = Component.extend({
|
|||
|
||||
initCodeMirror: task(function* () {
|
||||
let loader = this.get('lazyLoader');
|
||||
|
||||
yield RSVP.all([
|
||||
loader.loadScript('codemirror', 'assets/codemirror/codemirror.js')
|
||||
]);
|
||||
yield loader.loadScript('codemirror', 'assets/codemirror/codemirror.js');
|
||||
|
||||
scheduleOnce('afterRender', this, this._initCodeMirror);
|
||||
}),
|
||||
|
|
|
@ -1,4 +1,3 @@
|
|||
import $ from 'jquery';
|
||||
import RSVP from 'rsvp';
|
||||
import Service, {inject as service} from '@ember/service';
|
||||
import config from 'ghost-admin/config/environment';
|
||||
|
@ -22,31 +21,41 @@ export default Service.extend({
|
|||
},
|
||||
|
||||
loadScript(key, url) {
|
||||
if (this.get('testing')) {
|
||||
if (this.testing) {
|
||||
return RSVP.resolve();
|
||||
}
|
||||
|
||||
if (this.get(`scriptPromises.${key}`)) {
|
||||
// Script is already loaded/in the process of being loaded,
|
||||
// so return that promise
|
||||
return this.get(`scriptPromises.${key}`);
|
||||
if (this.scriptPromises[key]) {
|
||||
return this.scriptPromises[key];
|
||||
}
|
||||
|
||||
let ajax = this.get('ajax');
|
||||
let adminRoot = this.get('ghostPaths.adminRoot');
|
||||
let scriptPromise = new RSVP.Promise((resolve, reject) => {
|
||||
let {adminRoot} = this.ghostPaths;
|
||||
|
||||
let scriptPromise = ajax.request(`${adminRoot}${url}`, {
|
||||
dataType: 'script',
|
||||
cache: true
|
||||
let script = document.createElement('script');
|
||||
script.type = 'text/javascript';
|
||||
script.async = true;
|
||||
script.src = `${adminRoot}${url}`;
|
||||
|
||||
let el = document.getElementsByTagName('script')[0];
|
||||
el.parentNode.insertBefore(script, el);
|
||||
|
||||
script.addEventListener('load', () => {
|
||||
resolve();
|
||||
});
|
||||
|
||||
script.addEventListener('error', () => {
|
||||
reject(new Error(`${url} failed to load`));
|
||||
});
|
||||
});
|
||||
|
||||
this.set(`scriptPromises.${key}`, scriptPromise);
|
||||
this.scriptPromises[key] = scriptPromise;
|
||||
|
||||
return scriptPromise;
|
||||
},
|
||||
|
||||
loadStyle(key, url, alternate = false) {
|
||||
if (this.get('testing') || $(`#${key}-styles`).length) {
|
||||
if (this.testing || document.querySelector(`#${key}-styles`)) {
|
||||
return RSVP.resolve();
|
||||
}
|
||||
|
||||
|
@ -54,7 +63,7 @@ export default Service.extend({
|
|||
let link = document.createElement('link');
|
||||
link.id = `${key}-styles`;
|
||||
link.rel = alternate ? 'alternate stylesheet' : 'stylesheet';
|
||||
link.href = `${this.get('ghostPaths.adminRoot')}${url}`;
|
||||
link.href = `${this.ghostPaths.adminRoot}${url}`;
|
||||
link.onload = () => {
|
||||
if (alternate) {
|
||||
// If stylesheet is alternate and we disable the stylesheet before injecting into the DOM,
|
||||
|
@ -69,7 +78,7 @@ export default Service.extend({
|
|||
link.title = key;
|
||||
}
|
||||
|
||||
$('head').append($(link));
|
||||
document.querySelector('head').appendChild(link);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
|
@ -6,6 +6,7 @@ import {setupTest} from 'ember-mocha';
|
|||
|
||||
describe('Integration: Service: lazy-loader', function () {
|
||||
setupTest('service:lazy-loader', {integration: true});
|
||||
|
||||
let server;
|
||||
let ghostPaths = {
|
||||
adminRoot: '/assets/'
|
||||
|
@ -19,28 +20,32 @@ describe('Integration: Service: lazy-loader', function () {
|
|||
server.shutdown();
|
||||
});
|
||||
|
||||
it('loads a script correctly and only once', function () {
|
||||
it('loads a script correctly and only once', async function () {
|
||||
let subject = this.subject({
|
||||
ghostPaths,
|
||||
scriptPromises: {},
|
||||
testing: false
|
||||
});
|
||||
|
||||
server.get('/assets/test.js', function ({requestHeaders}) {
|
||||
expect(requestHeaders.Accept).to.match(/text\/javascript/);
|
||||
// first load should add script element
|
||||
await subject.loadScript('test', 'lazy-test.js')
|
||||
.then(() => {})
|
||||
.catch(() => {});
|
||||
|
||||
return [200, {'Content-Type': 'text/javascript'}, 'window.testLoadScript = \'testvalue\''];
|
||||
});
|
||||
expect(
|
||||
document.querySelectorAll('script[src="/assets/lazy-test.js"]').length,
|
||||
'no of script tags on first load'
|
||||
).to.equal(1);
|
||||
|
||||
return subject.loadScript('test-script', 'test.js').then(() => {
|
||||
expect(subject.get('scriptPromises.test-script')).to.exist;
|
||||
expect(window.testLoadScript).to.equal('testvalue');
|
||||
expect(server.handlers[0].numberOfCalls).to.equal(1);
|
||||
// second load should not add another script element
|
||||
await subject.loadScript('test', '/assets/lazy-test.js')
|
||||
.then(() => { })
|
||||
.catch(() => { });
|
||||
|
||||
return subject.loadScript('test-script', 'test.js');
|
||||
}).then(() => {
|
||||
expect(server.handlers[0].numberOfCalls).to.equal(1);
|
||||
});
|
||||
expect(
|
||||
document.querySelectorAll('script[src="/assets/lazy-test.js"]').length,
|
||||
'no of script tags on second load'
|
||||
).to.equal(1);
|
||||
});
|
||||
|
||||
it('loads styles correctly', function () {
|
||||
|
|
Loading…
Reference in a new issue