ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

New approach to MathJax-ifying equations 

closes LS-1601
flag=new_math_equation_handling

The previous approach was to replace the equation image with the
equation's LaTeX in canvas' backend, but not all user content sent
to the browser passes through UserContent.escape. Discussions
and legacy quiz questions included. The backend approach also suffered
from the an ugly visual where the LaTeX is displayed onscreen until
MathJax typesets it.

In a previous commit, I caught Discussion replies in apiUserContent
where the screenreader assistive mathml is injected into the DOM
adjacent to the image. That worked but we now had 2 places
where the replacement was taking place, and quiz questions are
still being missed.

A better approach is to handle it all in a central location, which
is with the code that detects math is on the page. The new approach
is to inject the LaTeX into the DOM adjacent to the image just before
MathJax does its processing, then removes the image when it finished.
This way the equation image is displayed to the user while MathJaX
does its work, and since we look for new math in a MutationObserver
watching the whole document, we never miss any equation images on the page.

Because we are looking for mutations anywhere on the page, there may
be nodes we want to ignore (e.g. the quiz timer). This is handled
by adding to the ignore_list css selector in main.js

test plan:
  - with the "Updated math equation handling" flag on
    (and optionally 'Support LaTex math equations almost everywhere")
  - double check that equations created with the rce equation editor
    are processed with mathjax all over canvas
  > expect equation images to be visible until replaced by MathJax
    typeset versions
  - Discussions:
    - reply to a discussion with an equation (inline and equation editor)
    > expect them to be typeset by mathjax
    - edit a reply and save
    > expect the the reply to have it's math processed by mathjax
  - Legacy Quizzes
    - create a quiz, set it so 1 question per page
    - add a couple questions with equations
    - preview the quiz, moving forward and back thru the questions
    > expect the questions go have their equations typeset by mathjax

Change-Id: I9e2ec4fd53de06748156bbd4adadac7e2b1e205f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/252222
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Peyton Craighill <pcraighill@instructure.com>
This commit is contained in:
Ed Schiebel 2020-11-07 09:41:14 -05:00
parent 29ca659af5
commit 0fc8ce19f7
7 changed files with 58 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
app/coffeescripts/str/apiUserContent.js
View File

@ -25,16 +25,7 @@ const apiUserContent = {
xsslint safeString.identifier mathml
*/
translateMathmlForScreenreaders($equationImage) {
if (ENV?.FEATURES?.new_math_equation_handling) {
// in most cases, the image will already have been replaced
// by UserContent.escape on the backend, but not for
// Discussion replies. Catch them here.
const equation_text = $equationImage.attr('data-equation-content')
const mathtex = document.createElement('span')
mathtex.setAttribute('class', 'math_equation_latex')
mathtex.textContent = `\\(${equation_text}\\)`
$equationImage.replaceWith(mathtex)
} else {
if (!ENV?.FEATURES?.new_math_equation_handling) {
// note, it is safe to treat the x-canvaslms-safe-mathml as html because it
// only ever gets put there by us (in Api::Html::Content::apply_mathml).
// Any user content that gets sent to the server will have the

10
app/jsx/main.js
View File

@ -103,11 +103,19 @@ ready(() => {
window.dispatchEvent(processNewMathEvent)
}, 0)
const ignore_list = '#quiz-elapsed-time' // comma-separated list of selectors to ignore
const processNewMathEvent = new Event(mathml.processNewMathEventName)
const observer = new MutationObserver((mutationList, _observer) => {
for (const m in mutationList) {
for (let m = 0; m < mutationList.length; ++m) {
if (mutationList[m]?.addedNodes?.length) {
const addedNodes = mutationList[m].addedNodes
for (let n = 0; n < addedNodes.length; ++n) {
const node = addedNodes[n]
if (node.nodeType !== Node.ELEMENT_NODE) return
if (node.parentElement?.querySelector(ignore_list)) return
}
window.dispatchEvent(processNewMathEvent)
return
}
}
})

9
lib/user_content.rb
View File

@ -58,14 +58,7 @@ module UserContent
find_equation_images(html) do |node|
equation = node['data-equation-content'] || node['alt']
next if equation.blank?
if use_updated_math_rendering
# replace the equation image with a span containing the
# LaTex, which MathJAX will typeset once we're in the browser
latex_span = Nokogiri::HTML::DocumentFragment.parse(
"<span class=\"math_equation_latex\">\\(#{equation}\\)</span>"
)
node.replace(latex_span)
else
if !use_updated_math_rendering
mathml = UserContent.latex_to_mathml(equation)
next if mathml.blank?

43
public/javascripts/mathml.js
View File

@ -63,11 +63,22 @@ const mathml = {
// wait until MathJAx is configured before calling the callback
cb?.()
})
window.MathJax.Hub.Register.MessageHook('Begin PreProcess', function(message) {
catchEquationImages(message[1])
})
window.MathJax.Hub.Register.MessageHook('End Math', function(message) {
removeStrayEquationImages(message[1])
message[1]
.querySelectorAll('.math_equation_latex')
.forEach(m => m.classList.add('fade-in-equation'))
})
// leaving this here so I don't have to keep looking up how to see all messages
// window.MathJax.Hub.Startup.signal.Interest(function (message) {
// console.log('>>> Startup:', message[0])
// })
// window.MathJax.Hub.signal.Interest(function(message) {
// console.log('>>> ', message[0])
// })
delete window.MathJaxIsLoading
},
dataType: 'script'
@ -89,6 +100,10 @@ const mathml = {
return true
}
if (document.querySelector('img.equation_image')) {
return true
}
if (ENV.FEATURES?.inline_math_everywhere) {
// look for latex the user may have entered w/o the equation editor by
// looking for mathjax's opening delimiters
@ -133,11 +148,37 @@ const mathml = {
processNewMathEventName: 'process-new-math'
}
function catchEquationImages(refnode) {
// find equation images and replace with inline LaTeX
const eqimgs = refnode.querySelectorAll('img.equation_image')
if (eqimgs.length > 0) {
eqimgs.forEach(img => {
const equation_text = img.getAttribute('data-equation-content')
const mathtex = document.createElement('span')
mathtex.setAttribute('class', 'math_equation_latex')
mathtex.textContent = `\\(${equation_text}\\)`
if (img.nextSibling) {
img.parentElement.insertBefore(mathtex, img.nextSibling)
} else {
img.parentElement.appendChild(mathtex)
}
})
return true
}
}
function removeStrayEquationImages(refnode) {
const eqimgs = refnode.querySelectorAll('img.equation_image')
eqimgs.forEach(img => img.parentElement.removeChild(img))
}
// TODO: if anyone firing the event ever needs a callback,
// push them onto an array, then pop and call in the handler
function handleNewMath() {
mathml.processNewMathOnPage()
if (ENV?.FEATURES?.new_math_equation_handling) {
mathml.processNewMathOnPage()
}
}
window.addEventListener('process-new-math', debounce(handleNewMath, 500))
export default mathml

4
public/javascripts/take_quiz.js
View File

@ -44,7 +44,7 @@ let lastAnswerSelected = null
let lastSuccessfulSubmissionData = null
let showDeauthorizedDialog
let quizSubmission = (function() {
const quizSubmission = (function() {
let timeMod = 0,
endAt = $('.end_at'),
endAtParsed = endAt.text() && new Date(endAt.text()),
@ -461,7 +461,7 @@ let quizSubmission = (function() {
$('.time_running').css('color', '#EA0611')
$timeRunningFunc().text(
I18n.t(
'Your browser connectivity may be slow or unstable. In spite of your browser\'s timer being disconnected, your answers will be recorded for an additional 5 minutes beyond the original time limit on this attempt.'
"Your browser connectivity may be slow or unstable. In spite of your browser's timer being disconnected, your answers will be recorded for an additional 5 minutes beyond the original time limit on this attempt."
)
)
return

4
spec/coffeescripts/str/apiUserContentSpec.js
View File

@ -41,10 +41,10 @@ test('moves mathml into a screenreader element', () => {
ok(output.includes('<span class="hidden-readable"><math '))
})
test('replaced math equation with LaTex if new_math_equation_handling flag is on', () => {
test('does not inject mathml if new_math_equation_handling flag is on', () => {
ENV.FEATURES = {new_math_equation_handling: true}
const output = apiUserContent.convert(mathml_html)
ok(output.includes('<span class="math_equation_latex">\\('))
ok(!output.includes('<span class="math_equation_latex">\\('))
})
test('mathml need not be screenreadered if editing content (this would start an update loop)', () => {

4
spec/javascripts/jsx/mathmlSpec.js
View File

@ -180,9 +180,7 @@ test('returns true if there is inline-delmited math', () => {
equal(mathml.isMathOnPage(), true)
})
QUnit.module('handles "process-new-math" events', {})
test('debounces event handler', assert => {
test('debounces "process-new-math" event handler', assert => {
const done = assert.async()
const spy = sinon.spy(mathml, 'processNewMathOnPage')
window.dispatchEvent(new Event('process-new-math'))