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

Separate the reporting of superscripts and subscripts from the report font attributes setting #10919

Merged
merged 4 commits into from
Mar 31, 2020

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Mar 30, 2020

Link to issue number:

Related to #3727.

Summary of the issue:

The reporting of superscript and subscript is controlled by the "report font attributes" setting (which is far too verbose for reading of mathematical/scientific documents using the <sup>/<sub> HTML tags).

Description of how this pull request fixes the issue:

Adds a new setting, "report superscripts and subscripts", which controls whether superscripted/subscripted text is read.

Testing performed:

With all possible combinations of "report superscripts and subscripts" and "report font attributes", tested that HTML documents in Chrome using the <sup> tag read correctly and that no non-selected formatting information is reported.

Known issues with pull request:

I'm not sure how to make this work in Braille – does NVDA even present superscripts/subscripts to Braille displays?

Change log entry:

== Changes ==

@LeonarddeR
Copy link
Collaborator

I think text position is ambiguous, as alignment also says something about the position of the text.
May be report script changes or report subscript and superscript?

@codeofdusk codeofdusk changed the title Separate the reporting of text position from the report font attributes setting Separate the reporting of superscripts and subscripts from the report font attributes setting Mar 31, 2020
@codeofdusk
Copy link
Contributor Author

I think text position is ambiguous, as alignment also says something about the position of the text.
May be report script changes or report subscript and superscript?

Fixed.

@AppVeyorBot
Copy link

See test results for failed build of commit 45f7a89490

@LeonarddeR
Copy link
Collaborator

SOrry for me not being explicit enough

I think that it is ok to call the option reportScript, as that's way shorter than reportSubScriptsAndSUperScripts. At first glance, you could leave the label as is, may be @feerrenrut has suggestions as what wording is used to cover both sub and superscript

Could you also add a config spec update that:

  • If report attributes is off (default), report script stays off
  • If report attributes is on, turn on script as well.

@codeofdusk
Copy link
Contributor Author

I prefer "report superscripts and subscripts" to "report script".

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Thanks @codeofdusk

@feerrenrut feerrenrut merged commit b5b6c79 into nvaccess:master Mar 31, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.1 milestone Mar 31, 2020
@feerrenrut feerrenrut modified the milestones: 2020.1, 2020.2 Mar 31, 2020
feerrenrut added a commit that referenced this pull request Mar 31, 2020
@josephsl
Copy link
Collaborator

josephsl commented Apr 2, 2020

Hi,

I'm afraid this introduced a regression: a KeyError exception is raised when NVDA+F is pressed to obtain formatting information.

Thanks.

@josephsl
Copy link
Collaborator

josephsl commented Apr 2, 2020

Traceback:

ERROR - scriptHandler.executeScript (20:38:42.761) - MainThread (3928):
error executing script: <bound method GlobalCommands.script_reportFormatting of <globalCommands.GlobalCommands object at 0x07620F10>> with gesture 'NVDA+f'
Traceback (most recent call last):
  File "scriptHandler.pyc", line 205, in executeScript
  File "globalCommands.pyc", line 1435, in script_reportFormatting
  File "globalCommands.pyc", line 1401, in _reportFormattingHelper
  File "textInfos\__init__.pyc", line 564, in getFormatFieldSpeech
  File "speech\__init__.pyc", line 2181, in getFormatFieldSpeech
KeyError: 'reportSuperscriptsAndSubscripts'

This is seen when pressing NVDA+F from browse mode documents. Thanks.

codeofdusk added a commit to codeofdusk/nvda that referenced this pull request Apr 2, 2020
feerrenrut pushed a commit that referenced this pull request Apr 3, 2020
Fixes  #10931
Follow on from #10919

The "report formatting script" (NVDA+f) was causing an error because the `reportSuperscriptsAndSubscripts` key was missing from the `formatConfig` dictionary used as a config section created in ` _reportFormattingHelper`. Updated the generation of this dictionary so this error is unlikely to be encountered again. Instead, now the risk is that any format option separated in the future my not be reported by the "report formatting script" when it should be.

Some superscript/subscript reporting was not separated. To fix this the superscript/subscript fetching has been separated in more situations (PowerPoint, UIA).
feerrenrut pushed a commit that referenced this pull request Apr 14, 2020
…. (PR #10979)

#10919 didn't cater to the Word object model, this adds support for older versions of Office, or those not using UIA.
@Adriani90
Copy link
Collaborator

Adriani90 commented Apr 30, 2020

Is it possible to exclude the reporting of baseline when pressing quick navigation key in browse mode? When I enabe superscripts and subscripts, NVDA always reports "base line" when I press quick navigation keys (i.e. b for buttons in the commenting area). This is quite annoying on some websites. This should be only reported when NVDA detects a mathematical content or if this is part of a text block while navigating with reading commands.

@codeofdusk
Copy link
Contributor Author

Most likely, open a separate issue and hopefully someone will take it.

feerrenrut pushed a commit that referenced this pull request May 4, 2020
In the Powerpoint appModule, the incorrect config key was used. Looking up reportSuperscriptAndSubscript (instead of the correct reportSuperscriptsAndSubscripts) resulted in an error, making certain functions unusable.

Config option introduced in: "Separate the reporting of superscripts and subscripts from the report font attributes setting #10919"
Regression introduced with: #10932

Fixes #11094.
@ABuffEr
Copy link
Contributor

ABuffEr commented Apr 4, 2021

Hi,
sorry, @codeofdusk , but I'm confused by this feature. If I enable or disable it, what should it happen reading
A<sup><em>i,j</em></sup>
rendered in a web page? I notice no differences in Chrome or Firefox, excluding what reported by @Adriani90.

seanbudd pushed a commit that referenced this pull request Apr 20, 2021
In IAccessible edit field such as WordPad, enabling "Superscript and subscript" reporting in Document formatting settings was not enough to have them reported. It was also required to have Font attributes reporting enabled.
This is due to a forgotten part of the code when reporting of superscript/subscript has been separated from reporting of the other attributes in #10919.

Description of how this pull request fixes the issue:
Added the missing 'if' statement
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.

8 participants