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.

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

General improvements to input/output errors #491

Merged
merged 3 commits into from
Jun 16, 2020
Merged

General improvements to input/output errors #491

merged 3 commits into from
Jun 16, 2020

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented May 13, 2020

This PR improves the formatting of 'Math output error' messages (to make them display like other errors, and to allow the contextual menu to be used, and to not cause errors with assistive-mml). The message is also stored in the inputData/outputData object for easier retrieval, if needed.

It also adds tooltips to the input and output errors to give the message for the error that occurred, and adds a similar tooltip to the noerrors output so that it is easier to get the TeX error that caused the problem. The SVG output jax's merror wrapper is modified to move the message to a <title> element where it will be displayed by the browser as a tool tip.

For noerrors output, it would be nice to be able to save the message in the MathItem's inputData object, but since the TeX input jax's formatError() method doesn't have access to the MathItem, that can't be done. It would be nice to change that (see #483).

…for the error message, and the same for NoErrors output. Save error messages in input/output data in the MathItem.
@dpvc dpvc added this to the v3.1.0 milestone May 13, 2020
Base automatically changed from pretty-components to pretty-code May 26, 2020 16:56
@dpvc dpvc changed the base branch from pretty-code to develop May 26, 2020 17:33
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

There's one problem I see with this use of the title element:

  • In CHTML I can see the tooltip. And the title attribute does not interfere with screen reading.
  • In SVG the title does not seem to have any effect: There is neither a tooltip nor is it usable as title for a screen reader. We could have it as a proper title element, i.e., not just an attribute, but then it would need to be above the aria-hidden attribute.

@dpvc
Copy link
Member Author

dpvc commented Jun 16, 2020

We could have it as a proper title element, i.e., not just an attribute, but then it would need to be above the aria-hidden attribute.

It actually already is a <title> element, it was just inside the wrong parent. This was written before the font-inheritance PR landed, and the error message in SVG was made up of the character paths. I put the <title> element in the background (yellow) rectangle, since it was more likely to be hovered over than the letters themselves. When the font-inheritance PR was merged, that made the error message into a <text> element, and that was on top of the background rectangle, so it blocked the rectangle (from a mouse-hovering standpoint) and the title wasn't displayed.

It turns out that moving the <title> to the <g> that represents the error itself will let it work both with and without font inheritance.

In CHTML is the tooltip read by screen readers? You say it doesn't interfere, but does that mean it is read or not read? I hadn't intended for it to be read. If you think it needs to be, then we can look into how to address that.

I've made a new merge to fix the SVG title issue. See if that works better for you.

Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

Just one line to remove. O/w OK.

@dpvc dpvc merged commit 8b8e9a5 into develop Jun 16, 2020
@dpvc dpvc deleted the better-errors branch June 16, 2020 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants