Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix and catch speech computation errors, add missing braille option #1053

Merged
merged 4 commits into from
Feb 10, 2024
Merged

Fix and catch speech computation errors, add missing braille option #1053

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
799ad89
Fix and catch speech computation errors, add missing braille option
zorkow Feb 8, 2024
1281331
Review suggestions and fixes SRE function name.
zorkow Feb 9, 2024
b4df886
Update for SRE version.
zorkow Feb 9, 2024
9f793a4
Simplifies state setting for attachSpeech.
zorkow Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions ts/a11y/semantic-enrich.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,21 @@ export function EnrichedMathItemMixin<N, T, D, B extends Constructor<AbstractMat
*/
public attachSpeech(document: MathDocument<N, T, D>) {
if (this.state() >= STATE.ATTACHSPEECH) return;
if (this.isEscaped || !document.options.enableEnrichment) {
this.state(STATE.ATTACHSPEECH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do any of the calls below cause a MathJax retry error? (That is, do any call mathjax.retryAfter()?)

If not, then you can do this.state(STATE.ATTACHSPEECH) above line 247, and then just return here and the other places where the state is set. That would avoid having to set the state in several places. But it only works if the rest of the code will always complete and not restart the typesetting via retryAfter().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not. The only place that triggers a retryAfter is in line 183:

MathJax-src/ts/a11y/semantic-enrich.ts

Line 183 in 799ad89

this.generatorPool.init(document.options, document.adaptor);

But that should have run before attachSpeech at all times.

return;
}
let [speech, braille] = this.existingSpeech();
let [newSpeech, newBraille] = ['', ''];
if (!speech || !braille ||
document.options.enableSpeech || document.options.enableBraille) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't catch this earlier, but I think this condition is wrong. That is, if document.options.enableSpeech is false, you should not generate speech even if !speech is true, and similarly for braille. So I think this should be

if ((!speech && document.options.enableSpeech) || (!bralle && document.options.enableBraille)) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Good catch.

[newSpeech, newBraille] = this.generatorPool.computeSpeech(
this.typesetRoot, this.toMathML(this.root, this));
try {
[newSpeech, newBraille] = this.generatorPool.computeSpeech(
this.typesetRoot, this.toMathML(this.root, this));
if (newSpeech) {
newSpeech = buildSpeech(newSpeech)[0];
}
} catch (_e) { }
}
speech = speech || newSpeech;
braille = braille || newBraille;
Expand Down Expand Up @@ -359,6 +368,7 @@ export function EnrichedMathDocumentMixin<N, T, D, B extends MathDocumentConstru
...BaseDocument.OPTIONS,
enableEnrichment: true,
enableSpeech: true,
enableBraille: true,
enrichError: (doc: EnrichedMathDocument<N, T, D>,
math: EnrichedMathItem<N, T, D>,
err: Error) => doc.enrichError(doc, math, err),
Expand Down