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

Don't use new math handling when editing quizzes 

closes LS-1639
flag=none

The change to SyllabusBehaviors puts code back that existed
before the new math handling was introduced, and should have
been behind its flag.

If the new_math_equation_handling flag is on, turn it off if
we're editing a quiz, and skip having the backend inject the
hidden mathml, which is part of the legacy equation handling.

test plan:
=== With the new math_equation_handling flag off ====
  - create a quiz, add a multiple choice question with an equation
    as an answer
  - save the question, save the quiz
  - edit the quiz, edit the question, do not edit the answer
  > look at the DOM. there should be no moe than 1
    <span class="hidden-readable">
    just after the equation image in the answer
  - save the question, save the quiz
  - preview the quiz
  > expect just 1 <span class="hidden-readable"> in the DOM just
    after the equation image
  - no combination of edit, save, edit, ... should cause > 1
    <span class="hidden-readable">
    after the equation image

=== with the new_math_equation_handling flag on ===
  - preview and edit the quiz you created with the flag off
  > expect it to look A-OK

  - create a new quiz
  - use the rce's equation editor to put an equation everywhere
    you can possible think of in a quiz
    - the text
    - answers
    - comments on the answers
  > expect the equations to look right no matter what
    - edit the quiz and all the places where there are equations
    > yep still ok
    - save the qustion
    > still ok
    - save the quiz
    > still ok
    - preview the quiz, to completion to see answer comments
    > looks good _and_ equations are mathjaxified
    - edit everything again
    > still looks good everywhere

Change-Id: I1319d007509f6e8cbc9c9af81e3939e365b0fa92
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/253507
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
This commit is contained in:
Ed Schiebel 2020-11-21 09:28:44 -05:00
parent 5ca7dbaef8
commit f18777afe0
7 changed files with 60 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/coffeescripts/behaviors/SyllabusBehaviors.js
View File

@ -27,6 +27,7 @@ import 'jquery.loadingImg'
import 'vendor/jquery.scrollTo'
import 'jqueryui/datepicker'
import easy_student_view from 'easy_student_view'
import mathml from 'mathml'
RichContentEditor.preloadRemoteModule()
@ -315,6 +316,15 @@ const bindToEditSyllabus = function(course_summary_enabled) {
$course_syllabus.loadingImage('remove').html(data.course.syllabus_body)
$course_syllabus.data('syllabus_body', data.course.syllabus_body)
$course_syllabus_details.hide()
if (!ENV.FEATURES.new_math_equation_handling) {
if (mathml.isMathMLOnPage()) {
if (mathml.isMathJaxLoaded()) {
mathml.reloadElement('content')
} else {
mathml.loadMathJax(undefined)
}
}
}
},
error(data) {

16
app/controllers/application_controller.rb
View File

@ -206,6 +206,7 @@ class ApplicationController < ActionController::Base
@js_env[:lolcalize] = true if ENV['LOLCALIZE']
@js_env[:rce_auto_save_max_age_ms] = Setting.get('rce_auto_save_max_age_ms', 1.day.to_i * 1000).to_i if @js_env[:rce_auto_save]
@js_env[:FEATURES][:new_math_equation_handling] = use_new_math_equation_handling?
end
end
@ -217,7 +218,7 @@ class ApplicationController < ActionController::Base
# put feature checks on Account.site_admin and @domain_root_account that we're loading for every page in here
# so altogether we can get them faster the vast majority of the time
JS_ENV_SITE_ADMIN_FEATURES = [:cc_in_rce_video_tray, :featured_help_links, :rce_lti_favorites, :new_math_equation_handling].freeze
JS_ENV_SITE_ADMIN_FEATURES = [:cc_in_rce_video_tray, :featured_help_links, :rce_lti_favorites].freeze
JS_ENV_ROOT_ACCOUNT_FEATURES = [
:direct_share, :assignment_bulk_edit, :responsive_awareness, :recent_history,
:responsive_misc, :product_tours, :module_dnd, :files_dnd, :unpublished_courses, :bulk_delete_pages,
@ -2133,7 +2134,7 @@ class ApplicationController < ActionController::Base
return nil unless str
return str.html_safe unless str.match(/object|embed|equation_image/)
UserContent.escape(str, request.host_with_port)
UserContent.escape(str, request.host_with_port, use_new_math_equation_handling?)
end
helper_method :user_content
@ -2151,7 +2152,7 @@ class ApplicationController < ActionController::Base
is_public: is_public
).processed_url
end
UserContent.escape(rewriter.translate_content(str), request.host_with_port)
UserContent.escape(rewriter.translate_content(str), request.host_with_port, use_new_math_equation_handling?)
end
helper_method :public_user_content
@ -2194,6 +2195,15 @@ class ApplicationController < ActionController::Base
true
end
# the way classic quizzes copies question data from the page into the
# edit form causes the elements added for a11y to get duplicated
# and other misadventures that caused 4 hotfixes in 3 days.
# Let's just not use the new math handling there.
def use_new_math_equation_handling?
Account.site_admin.feature_enabled?(:new_math_equation_handling) &&
!(params[:controller] == "quizzes/quizzes" && params[:action] == "edit")
end
def destroy_session
logger.info "Destroying session: #{session[:session_id]}"
@pseudonym_session.destroy rescue true

3
app/helpers/quizzes_helper.rb
View File

@ -411,7 +411,8 @@ module QuizzesHelper
html = hash_get(hash, "#{field}_html".to_sym)
if html
UserContent.escape(Sanitize.clean(html, CanvasSanitize::SANITIZE))
use_new_math = Account.site_admin.feature_enabled?(:new_math_equation_handling) && action_name != "edit"
UserContent.escape(Sanitize.clean(html, CanvasSanitize::SANITIZE), nil, use_new_math)
else
hash_get(hash, field)
end

15
lib/user_content.rb
View File

@ -22,7 +22,11 @@ require 'ritex'
require 'securerandom'
module UserContent
def self.escape(str, current_host = nil)
def self.escape(
str,
current_host = nil,
use_updated_math_rendering = Account.site_admin.feature_enabled?(:new_math_equation_handling)
)
html = Nokogiri::HTML::DocumentFragment.parse(str)
find_user_content(html) do |obj, uc|
uuid = SecureRandom.uuid
@ -58,7 +62,14 @@ module UserContent
find_equation_images(html) do |node|
equation = node['data-equation-content'] || node['alt']
next if equation.blank?
if !Account.site_admin.feature_enabled?(:new_math_equation_handling)
# there are places in canvas (e.g. classic quizzes) that
# inadvertently saved the hidden-readable span, causing
# them to multiply everytime the entity is edited.
# Strip the ones that shouldn't be there before adding a new one
node.next.remove while node.next && node.next['class'] == 'hidden-readable'
if !use_updated_math_rendering
mathml = UserContent.latex_to_mathml(equation)
next if mathml.blank?

14
public/javascripts/instructure.js
View File

@ -113,11 +113,15 @@ export function enhanceUserContent() {
.each(function() {
const $this = $(this)
$this.find('img').each((i, img) => {
const handleWidth = () =>
$(img).css(
'maxWidth',
Math.min($content.width(), $this.width(), $(img).width() || img.naturalWidth)
)
const handleWidth = () => {
const maxw = Math.min($content.width(), $this.width(), $(img).width() || img.naturalWidth)
if (maxw > 0) {
$(img).css(
'maxWidth',
Math.min($content.width(), $this.width(), $(img).width() || img.naturalWidth)
)
}
}
if (img.naturalWidth === 0) {
img.addEventListener('load', handleWidth)
} else {

3
public/javascripts/mathml.js
View File

@ -124,8 +124,9 @@ const mathml = {
return false
},
// legacy api
isMathMLOnPage() {
return this.isMathOnPage() // just in case I missed a place it's being used
return this.isMathOnPage()
},
isMathJaxLoaded() {

11
spec/lib/user_content_spec.rb
View File

@ -206,5 +206,16 @@ describe UserContent do
"</ul></div>"
expect(html).to eq(expected)
end
it "strips existing mathml before adding any new" do
string = "<div><img class='equation_image' data-equation-content='\int f(x)/g(x)'/>"\
"<span class=\"hidden-readable\"><math>3</math></span></div>"
html = UserContent.escape(string)
expected = "<div>\n"\
"<img class=\"equation_image\" data-equation-content=\"int f(x)/g(x)\"><span class=\"hidden-readable\"><math xmlns=\"http://www.w3.org/1998/Math/MathML\" display=\"inline\"><mi>i</mi><mi>n</mi><mi>t</mi><mi>f</mi><mo stretchy=\"false\">(</mo><mi>x</mi><mo stretchy=\"false\">)</mo><mo>/</mo><mi>g</mi><mo stretchy=\"false\">(</mo><mi>x</mi><mo stretchy=\"false\">)</mo></math></span>\n"\
"</div>"
expect(html).to eq(expected)
end
end
end