EDUCATOR-3764 Mathjax accessibility files #19365
Conversation
DawoudSheraz
force-pushed
the
dsheraz/educator_3764
branch
from
99a704e
to
3c6f515
Compare
DawoudSheraz
force-pushed
the
dsheraz/educator_3764
branch
7 times, most recently
from
a758344
to
116e8e7
Compare
|
Looks like the failing tests here are related to answering problems, have we investigated deeper into why they're failing instead of continuing to rerun them? |
DawoudSheraz
force-pushed
the
dsheraz/educator_3764
branch
from
116e8e7
to
a22a983
Compare
|
@fysheets The tests are unrelated to the changes. I will try to rebase and run them again. If they are still failing, then we will have to take a look at what is going on. Also, they are failing randomly, so not sure what is happening. |
|
jenkins run quality |
There was a problem hiding this comment.
@noraiz-anwar Can you find some time today and sit with @DawoudSheraz and test this on sandbox?
| @@ -59,13 +59,12 @@ | |||
| this.renderAttrs(); | |||
| this.$('span.timeago').timeago(); | |||
| this.convertMath(); | |||
| this.$('.post-body'); | |||
There was a problem hiding this comment.
What are these two lines doing? why they are removed?
There was a problem hiding this comment.
These two lines weren't doing anything.On inspecting these two lines further in the console, they were only returning the element containing the mentioned css (just like find() function).
|
Thank you for the descriptive PR. 👍 |
|
@DawoudSheraz please let me know if you need any help regarding this. |
DawoudSheraz
force-pushed
the
dsheraz/educator_3764
branch
from
a22a983
to
e954eb0
Compare
|
jenkins run lettuce |
|
Your PR has finished running tests. There were no failures. |
|
🎆 |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Wednesday, December 19, 2018. |
|
The lettuce tests related to resetting formula problems have failed 5 times on master in the 1 day since this was merged, after more than a week of clean builds; can someone who worked on this please take a look and try to resolve that? |
|
EdX Release Notice: This PR has been deployed to the production environment. |
EDUCATOR-3764
Description
Mathjax provides with the accessibility features to allow access to the rendered content in every possible ways. In current scenario, the files required for the a11y are loaded manually. There is need to enable those files automatically whenever the Mathjax is loaded. This PR addresses the configuration to load the a11y files for the Mathjax by upgrading it to 2.7.5 and adding the required configuration.
Change Impact
Adding the MathJax a11y files led to the discovery of a problem in the Discussion component. Surprisingly, the problem has been there in first place for quite a long time, but only came under the observation after the inclusion of MathJax a11y. The problem was that whenever a response (to a discussion forum) was added/edited, Math Processing Error occurred, which halted the further MathJax Processing on the page, unless page was reloaded.
Problem Identification
Identifying the problem was straightforward, but locating the exact point of the origin was cumbersome. When a response is added, the contents from the input area are taken, and appended to the already present response list. During that process, whenever the render of the response model was called, a Typeset call was queued inside the MathJax queue (Discussion util convertMath function was called inside render). Queue & TypeSet being async performed on their own, while the page DOM modified to append the new response. When a response is successfully appended, the backbone model was updated via set call, which in turn generated relevant events. One of the triggered events called the render method again, which led to the additional typeset calls. The problem here is that previous typesetting didn't complete, and DOM changed. The amalgam of async and sync operations is problematic for MathJax operations, as mentioned in one of the MathJax reported issues (For further study, take a look at the question as well).
Problem Resolution
Given the coupling of the various discussion component models, it was rather difficult to convert all the operations either sync or async. One of the possible fixes was to TypeSet the response when the underlying operation finished properly. This meant moving the TypeSet call out from convertMath function, and calling the TypeSet explicitly whenever required. This somehow violated the DRY principle as the same thing was mentioned in the multiple places, but in this case, explicit calls were better than the implicit problematic calls. Hence, for the responses, whenever any operation (adding, editing, etc.) finished, only then Typeset was called. This solved the underlying problem.
Testing
Following Mathjax expressions' speech output has been verified:
The resultant output expressions were checked using ChromeVox
Sandbox
Reviewers
Post Review