Use MathML attributes for PDFs read in Adobe Acrobat

[NSoiffer]

Link to issue number:

This fixes #17980

Summary of the issue:

For PDF, the code was not grabbing MathML attributes needed for speech.

Description of user facing changes

The speech for math, and rarely the braille (e.g., bevelled fractions in Nemeth), was not always correct due to attributes not being picked up and the defaults being used.

Description of development approach

Unfortunately, the PDF interface does not allow grabbing all the attributes. Instead, one must ask for each attribute individually. Most attributes don't affect speech or braille, so it is not necessary to get them. I looked at what MathCAT used and added those. In particular, the following are picked up:

		id = node.GetID()
		if id:
			yield f' id="{id}"'
		yield getMathMLAttributes(node, ["intent", "arg"])
		if tag == "mi" or tag == "mn" or tag == "mo" or tag == "mtext":
			yield getMathMLAttributes(node, ["mathvariant"])
		elif tag == "mfenced":
			yield getMathMLAttributes(node, ["open", "close", "separators"])
		elif tag == "menclose":
			yield getMathMLAttributes(node, ["notation"])
		elif tag == "annotation-xml" or tag == "annotation":
			yield getMathMLAttributes(node, ["encoding"])
		elif tag == "ms":
			yield getMathMLAttributes(node, ["open", "close"])

Note: intent and arg are new to MathML 4 and are aimed at improving speech for math.

Testing strategy:

There is a test file in the issue. It tests intent. I asked David Carlisle, a LaTeX developer, to generate some (fully tagged) PDF examples that use more than intent (which is what pdftex will generate on its own in some cases. He gave me a sample that displays poorly, but has some attrs hacked into it. There is a log statement (at debug level) that shows the MathML that is gathered up. The MathML picked was correct. The speech was also as expected given the MathML.

I don't know how one would create a unit or system test for this. The issue contains a PDF example which can be tested (issue describes the expected result).

Known issues with pull request:

None.

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

@coderabbitai summary

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: License check.
• PASS: Unit tests.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/84xae4qigf6drxwq/artifacts/output/nvda_snapshot_pr17984-36103,d7c86a33.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 1.2,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  BUILD_END 18.0,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.4,
  TEST_START 0.0,
  TEST_END 18.5,
  FINISH_END 0.1

See test results for failed build of commit d7c86a33bf

[codeofdusk]

@NSoiffer Should this target beta (i.e. is it a release-blocking bug in #17276)?

[NSoiffer]

It is a bug that results in incorrect speech and possibly incorrect braille if one of the attributes for the elements listed above is present. For example, if someone uses

<mi mathvariant="double-struck">C</mi>

then instead of hearing either "the complex numbers" or "double struck cap C", they will just hear "cap C". Similarly, the braille will be wrong as it will be missing a script indicator. Or as in the bug report, they won't hear the equation label as a label ("line 2 with label 2 ...") and I think they will hear the label as just data which is very confusing.

My feeling this is a pretty serious bug and should be part of the beta.

[codeofdusk]

In which case, please change the base branch on GitHub and rebase your branch accordingly.

[NSoiffer]

I think I have this rebased to the beta and made all the changes suggested. This is at the edge of my github knowledge, so hopefully it is ok.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: License check.
• PASS: Unit tests.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/0tjadl64aup472s2/artifacts/output/nvda_snapshot_pr17984-36160,e82e2d78.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 1.3,
  INSTALL_END 1.0,
  BUILD_START 0.0,
  BUILD_END 19.6,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.4,
  TEST_START 0.0,
  TEST_END 19.2,
  FINISH_END 0.2

See test results for failed build of commit e82e2d78b3

[seanbudd]

This has been retargeted to beta, but not rebased off beta. It includes commits from master which cannot be merged. You need to drop all the commits from master in the rebase

[SaschaCowley]

@NSoiffer do you need help correctly rebasing this?

[davidcarlisle]

@NSoiffer do you need help correctly rebasing this?

Neil is traveling and mostly offline for a while, if you can handle this at your side I'm sure it would be appreciated.

@seanbudd please see #17984 (comment) where any changes are pre-authorized.

I have tested Neil's change and confirm it works but don't have an nvda build environment set up so can't easily help clear any remaining issues on this PR.

[davidcarlisle]

@SaschaCowley @seanbudd thanks for picking this up in Neil's absence. With 2025.1 we will be able to make mathematical PDF that may be read in acrobat and foxit with the mathematical content read to the same level as web pages, this PR is a last missing piece for acrobat. I'm so happy to see it merged.