Wait for tinyrce to initialize itself before accessing the editor

closes LA-620
flag=la_620_old_rce_init_fix

test plan:
  - Turn off rce_enhancements
  - Create an assignment and edit in some desciption
  - Save
  - Edit
    - if you are recreating the original issue, loading
      the edit page will cause a js error, the sidebar
      will not load and you will not be able to save
      the assignemnt. Ideally, you can reproduce this
      before moving on.
  - turn on the la_620_old_rce_init_fix flag
  - repeatedly refresh the assignment's edit page
  > expect no js error and the sidebar loads every time

Change-Id: Ie0217e4aba1b61a3cd5986e35da7aa62ea1297b6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/228299
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Clint Furse <cfurse@instructure.com>
This commit is contained in:
Ed Schiebel 2020-02-27 18:01:46 -05:00
parent 3e0f239479
commit 2e3ee0bc82
5 changed files with 75 additions and 16 deletions

View File

@ -171,7 +171,8 @@ class ApplicationController < ActionController::Base
show_feedback_link: show_feedback_link?
},
FEATURES: {
assignment_attempts: Account.site_admin.feature_enabled?(:assignment_attempts)
assignment_attempts: Account.site_admin.feature_enabled?(:assignment_attempts),
la_620_old_rce_init_fix: Account.site_admin.feature_enabled?(:la_620_old_rce_init_fix)
}
}
@js_env[:current_user] = @current_user ? Rails.cache.fetch(['user_display_json', @current_user].cache_key, :expires_in => 1.hour) { user_display_json(@current_user, :profile, [:avatar_is_fallback]) } : {}

View File

@ -25,6 +25,24 @@ import polyfill from './polyfill'
import splitAssetString from 'compiled/str/splitAssetString'
import closedCaptionLanguages from '../closedCaptionLanguages'
async function getTinyInstance(remoteEditor) {
// eslint-disable-next-line no-unused-vars
return new Promise((resolve, reject) => {
let ed = remoteEditor.mceInstance()
if (ed) {
resolve(ed)
} else {
const iid = setInterval(() => {
ed = remoteEditor.mceInstance()
if (ed) {
clearInterval(iid)
resolve(ed)
}
}, 1000)
}
})
}
function getTrayProps() {
if (!ENV.context_asset_string) {
return null
@ -69,13 +87,26 @@ const RCELoader = {
const renderingTarget = this.getRenderingTarget(textarea, tinyMCEInitOptions.getRenderingTarget)
const propsForRCE = this.createRCEProps(textarea, tinyMCEInitOptions)
this.loadRCE(RCE => {
RCE.renderIntoDiv(renderingTarget, propsForRCE, remoteEditor => {
remoteEditor
.mceInstance()
.on('init', () => callback(textarea, polyfill.wrapEditor(remoteEditor)))
if (ENV?.FEATURES?.la_620_old_rce_init_fix) {
// in the old rce using an old tinymce, it's possible renderIntoDiv's callback is called
// before tinymce adds the new editor to its internal list, and remoteEditor.mceInstance()
// wil return null. Waiting on getTinyInstance works around that.
// Put it back to the simple `else` case once the old rce is no longer used
this.loadRCE(RCE => {
RCE.renderIntoDiv(renderingTarget, propsForRCE, async remoteEditor => {
const tinyed = await getTinyInstance(remoteEditor)
tinyed.on('init', () => callback(textarea, polyfill.wrapEditor(remoteEditor)))
})
})
})
} else {
this.loadRCE(RCE => {
RCE.renderIntoDiv(renderingTarget, propsForRCE, remoteEditor => {
remoteEditor
.mceInstance()
.on('init', () => callback(textarea, polyfill.wrapEditor(remoteEditor)))
})
})
}
},
loadSidebarOnTarget(target, callback) {

View File

@ -37,3 +37,11 @@ syllabus_course_summary_option:
applies_to: SiteAdmin
display_name: Syllabus Course Summary Option
description: Make the Course Summary (list of assignments and events) optional in the syllabus
la_620_old_rce_init_fix:
state: hidden
display_name: RCE Initialization Fix
description: |-
Under some conditions, the legacy RCE fails to initialize because tinymce hasn't added the
editor instance to its internal list when the RCE requires it.
applies_to: SiteAdmin
root_opt_in: true

View File

@ -28,6 +28,7 @@ export default {
domain_root_account_cache_key: 'accounts/1-20111117224337',
context_cache_key: 'users/1-20111116001415',
PERMISSIONS: {},
FEATURES: {},
...options
}
},

View File

@ -135,7 +135,7 @@ test('passes the textarea height into tinyOptions', () => {
const taHeight = '123'
const textarea = {offsetHeight: taHeight}
const opts = {defaultContent: 'default text'}
const props = RCELoader.createRCEProps(textarea, opts)
RCELoader.createRCEProps(textarea, opts)
equal(opts.tinyOptions.height, taHeight)
})
@ -158,17 +158,35 @@ test('renders with rce', function() {
ok(this.rce.renderIntoDiv.calledWith(this.$div.get(0)))
})
test('yields editor to callback', function() {
const cb = sinon.spy()
test('yields editor to callback,', function(assert) {
const done = assert.async()
const cb = (textarea, rce) => {
equal(textarea, this.$textarea.get(0))
equal(rce, this.editor)
done()
}
RCELoader.loadOnTarget(this.$div, {}, cb)
ok(cb.calledWith(this.$textarea.get(0), this.editor))
})
test('ensures yielded editor has call and focus methods', function() {
const cb = sinon.spy()
test('yields editor to callback, la_620_old_rce_init_fix feature enabled', function(assert) {
ENV.FEATURES.la_620_old_rce_init_fix = true
const done = assert.async()
const cb = (textarea, rce) => {
equal(textarea, this.$textarea.get(0))
equal(rce, this.editor)
done()
}
RCELoader.loadOnTarget(this.$div, {}, cb)
})
test('ensures yielded editor has call and focus methods', function(assert) {
const done = assert.async()
const cb = (textarea, rce) => {
equal(typeof rce.call, 'function')
equal(typeof rce.focus, 'function')
done()
}
RCELoader.loadOnTarget(this.$div, {}, cb)
equal(typeof this.editor.call, 'function')
equal(typeof this.editor.focus, 'function')
})
QUnit.module('loadSidebarOnTarget', {
@ -209,7 +227,7 @@ test('yields sidebar to callback', function() {
})
test('ensures yielded sidebar has show and hide methods', function() {
const cb = sinon.spy()
const cb = () => {}
RCELoader.loadSidebarOnTarget(this.$div, cb)
equal(typeof this.sidebar.show, 'function')
equal(typeof this.sidebar.hide, 'function')