search

nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2k stars 621 forks source link

Add Unicode Normalization to speech and braille #16521

Closed LeonarddeR closed 3 weeks ago

LeonarddeR commented 1 month ago

Link to issue number:

Closes #16466

Summary of the issue:

Several speech synthesizers and braille tables are unable to speak or braille some characters, such as ligatures ("ij") or decomposed characters (latin letters with a modifier to add acute, diaeresis, etc.). Also, italic or bold Unicode characters can't be spoken or brailled by default.

Description of user facing changes

None by default. If unicode normalization is enabled for speech, speech output for objects and text navigation is normalized. For braille, normalization is applied for all braille output.

The reason for speech to apply normalization only for objects and text navigation is chosen on purpose, as for individual character navigation or text selection, we really want to pass the original character to the synthesizer. If we don't Unicode bold and italic characters are read as their normalized counterparts, which makes it impossible to distinguish them. This problem is less relevant when working with braille.

Description of development approach

  1. Added UnicodeNormalizationOffsetConverter to textUtils with tests. It stores the normalized version of a given string and based on diffing, calculates offset mappings between the original and normalized strings.
  2. Processes output using UnicodeNormalizationOffsetConverter when translating braille with normalization on. The several mappings (braille to raw pos, raw to braille pos) are processed to account for normalization.
  3. Added normalization to getTextInfoSpeech and getPropertiesSpeech.

Testing strategy:

  1. Unit tests
  2. Working with braille, testing cursor positioning and cursor routing on the several strings in the unit tests.

Known issues with pull request:

None known.

Code Review Checklist:

Summary by CodeRabbit

Simon818 commented 1 month ago

I'm running a build that contains this PR and am receiving an error pretty frequently when trying to navigate with speech normalization enabled.

Could not execute function event_gainFocus defined in appModules.nvda module; kwargs: {} Traceback (most recent call last): File "eventHandler.pyc", line 127, in next File "NVDAObjects__init__.pyc", line 1306, in event_gainFocus File "NVDAObjects__init__.pyc", line 1157, in reportFocus File "speech\speech.pyc", line 694, in speakObject File "speech\speech.pyc", line 733, in getObjectSpeech File "speech\speech.pyc", line 660, in getObjectPropertiesSpeech File "speech\speech.pyc", line 1975, in getPropertiesSpeech File "speech\speech.pyc", line 1975, in File "textUtils.pyc", line 568, in unicodeNormalize TypeError: normalize() argument 2 must be str, not CharacterModeCommand

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "eventHandler.pyc", line 127, in next File "appModules\nvda.pyc", line 211, in event_gainFocus File "eventHandler.pyc", line 134, in next File "extensionPoints\util.pyc", line 216, in callWithSupportedKwargs File "NVDAObjects__init__.pyc", line 1306, in event_gainFocus File "NVDAObjects__init__.pyc", line 1157, in reportFocus File "speech\speech.pyc", line 694, in speakObject File "speech\speech.pyc", line 733, in getObjectSpeech File "speech\speech.pyc", line 660, in getObjectPropertiesSpeech File "speech\speech.pyc", line 1975, in getPropertiesSpeech File "speech\speech.pyc", line 1975, in File "textUtils.pyc", line 568, in unicodeNormalize TypeError: normalize() argument 2 must be str, not CharacterModeCommand

CC @codeofdusk

LeonarddeR commented 1 month ago

It looks like there is an erroneous type annotation in getPropertiesSpeech that miss-leaded me. getKeyboardShortcutsSpeech can now also return character mode commands, so the getPropertiesSpeech doesn't only return strings now.

LeonarddeR commented 1 month ago

@Qchristensen Could you have a look at the proposed documentation changes?

seanbudd commented 1 month ago

Hi - testing code rabbit on this PR for an AI review. I've tried to adjust it to only pick up serious issues rather than linting. Apologies for the noise if it picks up junk.

@coderabbitai review

coderabbitai[bot] commented 1 month ago

@seanbudd, I have identified the issue causing the error reported by Simon818. The getPropertiesSpeech function attempts to normalize all items in the textList, but some items are not strings, leading to a type error. Here is the updated code for the getPropertiesSpeech function to ensure only strings are passed to unicodeNormalize:

def getPropertiesSpeech(  # noqa: C901
        reason: OutputReason = OutputReason.QUERY,
        **propertyValues
) -> SpeechSequence:
    textList: SpeechSequence = []
    name: Optional[str] = propertyValues.get('name')
    if name:
        textList.append(name)
    if 'role' in propertyValues:
        role: controlTypes.Role = propertyValues['role']
        speakRole=True
    elif '_role' in propertyValues:
        speakRole=False
        role: controlTypes.Role = propertyValues['_role']
    else:
        speakRole=False
        role=controlTypes.Role.UNKNOWN
    role = controlTypes.Role(role)
    value: Optional[str] = propertyValues.get('value') if role not in controlTypes.silentValuesForRoles else None
    cellCoordsText: Optional[str] = propertyValues.get('cellCoordsText')
    rowNumber = propertyValues.get('rowNumber')
    columnNumber = propertyValues.get('columnNumber')
    includeTableCellCoords = propertyValues.get('includeTableCellCoords', True)

    if role == controlTypes.Role.CHARTELEMENT:
        speakRole = False
    roleText: Optional[str] = propertyValues.get('roleText')
    if (
        speakRole
        and (
            roleText
            or reason not in (
                OutputReason.SAYALL,
                OutputReason.CARET,
                OutputReason.FOCUS,
                OutputReason.QUICKNAV
            )
            or not (
                name
                or value
                or cellCoordsText
                or rowNumber
                or columnNumber
            )
            or role not in controlTypes.silentRolesOnFocus
        )
        and (
            role != controlTypes.Role.MATH
            or reason not in (
                OutputReason.CARET,
                OutputReason.SAYALL
            )
        )):
        textList.append(roleText if roleText else role.displayString)
    if value:
        textList.append(value)
    states = propertyValues.get('states')
    realStates=propertyValues.get('_states',states)
    negativeStates=propertyValues.get('negativeStates',set())
    if states is not None or negativeStates:
        if states is None:
            states = set()
        labelStates = controlTypes.processAndLabelStates(role, realStates, reason, states, negativeStates)
        textList.extend(labelStates)
    description: Optional[str] = propertyValues.get('description')
    if description:
        textList.append(description)
    keyboardShortcut: Optional[str] = propertyValues.get('keyboardShortcut')
    textList.extend(getKeyboardShortcutsSpeech(keyboardShortcut))
    if includeTableCellCoords and cellCoordsText:
        textList.append(cellCoordsText)
    if cellCoordsText or rowNumber or columnNumber:
        tableID = propertyValues.get("_tableID")
        sameTable = (tableID and tableID == _speechState.oldTableID)
        if tableID and not sameTable:
            _speechState.oldTableID = tableID
        rowSpan = propertyValues.get("rowSpan") or 1
        columnSpan = propertyValues.get("columnSpan") or 1
        if rowNumber and (
            not sameTable
            or rowNumber != _speechState.oldRowNumber
            or rowSpan != _speechState.oldRowSpan
        ):
            rowHeaderText: Optional[str] = propertyValues.get("rowHeaderText")
            if rowHeaderText:
                textList.append(rowHeaderText)
            if includeTableCellCoords and not cellCoordsText: 
                rowNumberTranslation: str = _("row %s") % rowNumber
                textList.append(rowNumberTranslation)
                if rowSpan>1 and columnSpan<=1:
                    rowSpanAddedTranslation: str = _("through {endRow}").format(endRow=rowNumber + rowSpan - 1)
                    textList.append(rowSpanAddedTranslation)
            _speechState.oldRowNumber = rowNumber
            _speechState.oldRowSpan = rowSpan
        if columnNumber and (
            not sameTable
            or columnNumber != _speechState.oldColumnNumber
            or columnSpan != _speechState.oldColumnSpan
        ):
            columnHeaderText: Optional[str] = propertyValues.get("columnHeaderText")
            if columnHeaderText:
                textList.append(columnHeaderText)
            if includeTableCellCoords and not cellCoordsText:
                colNumberTranslation: str = _("column %s") % columnNumber
                textList.append(colNumberTranslation)
                if columnSpan>1 and rowSpan<=1:
                    colSpanAddedTranslation: str = _("through {endCol}").format(endCol=columnNumber + columnSpan - 1)
                    textList.append(colSpanAddedTranslation)
            _speechState.oldColumnNumber = columnNumber
            _speechState.oldColumnSpan = columnSpan
        if includeTableCellCoords and not cellCoordsText and rowSpan>1 and columnSpan>1:
            rowColSpanTranslation: str = _("through row {row} column {column}").format(
                row=rowNumber + rowSpan - 1,
                column=columnNumber + columnSpan - 1
            )
            textList.append(rowColSpanTranslation)
    rowCount=propertyValues.get('rowCount',0)
    columnCount=propertyValues.get('columnCount',0)
    rowAndColumnCountText = _rowAndColumnCountText(rowCount, columnCount)
    if rowAndColumnCountText:
        textList.append(rowAndColumnCountText)
    if rowCount or columnCount:
        _speechState.oldTableID = None

    isCurrent = propertyValues.get('current', controlTypes.IsCurrent.NO)
    if isCurrent != controlTypes.IsCurrent.NO:
        textList.append(isCurrent.displayString)

    hasDetails = propertyValues.get('hasDetails', False)
    if hasDetails:
        detailsRoles: _AnnotationRolesT = propertyValues.get("detailsRoles", tuple())
        if detailsRoles:
            roleStrings = (role.displayString if role else _("details") for role in detailsRoles)
            for roleString in roleStrings:
                textList.append(
                    _("has %s") % roleString
                )
        else:
            textList.append(
                _("has details")
            )

    placeholder: Optional[str] = propertyValues.get('placeholder', None)
    if placeholder:
        textList.append(placeholder)
    indexInGroup=propertyValues.get('positionInfo_indexInGroup',0)
    similarItemsInGroup=propertyValues.get('positionInfo_similarItemsInGroup',0)
    if 0<indexInGroup<=similarItemsInGroup:
        itemPosTranslation: str = _("{number} of {total}").format(
            number=indexInGroup,
            total=similarItemsInGroup
        )
        textList.append(itemPosTranslation)
    if 'positionInfo_level' in propertyValues:
        level=propertyValues.get('positionInfo_level',None)
        role=propertyValues.get('role',None)
        if level is not None:
            levelTranslation: str = _('level %s') % level
            if (
                role in (controlTypes.Role.TREEVIEWITEM, controlTypes.Role.LISTITEM)
                and level != _speechState.oldTreeLevel
            ):
                textList.insert(0, levelTranslation)
                _speechState.oldTreeLevel = level
            else:
                textList.append(levelTranslation)

    errorMessage: str | None = propertyValues.get("errorMessage", None)
    if errorMessage:
        textList.append(errorMessage)
    if config.conf["speech"]["unicodeNormalization"]:
        textList = [
            unicodeNormalize(t) if isinstance(t, str) else t
            for t in textList
        ]
    types.logBadSequenceTypes(textList)
    return textList

This update ensures that only strings are passed to unicodeNormalize, preventing the type error.

Analysis chain --- --- --- ---
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for the definition of the getPropertiesSpeech function in the codebase. ast-grep --lang python --pattern $'def getPropertiesSpeech($_):\n $$$' # Search for the usage of the unicodeNormalize function in the codebase. ast-grep --lang python --pattern $'def unicodeNormalize($_):\n $$$' # Search for the CharacterModeCommand class or type in the codebase. ast-grep --lang python --pattern $'class CharacterModeCommand($_):\n $$$' ``` Length of output: 1517 --- --- Script: ```shell #!/bin/bash # Search for the definition of the getPropertiesSpeech function in the speech module. ast-grep --lang python --pattern $'def getPropertiesSpeech($_):\n $$$' source/speech # Search for the definition of the unicodeNormalize function in the textUtils module. ast-grep --lang python --pattern $'def unicodeNormalize($_):\n $$$' source/textUtils ``` Length of output: 239 --- --- Script: ```shell #!/bin/bash # Locate the textUtils module in the repository. fd textUtils.py # Locate the getPropertiesSpeech function in the repository. ast-grep --lang python --pattern $'def getPropertiesSpeech($_):\n $$$' ``` Length of output: 140 --- --- Script: ```shell #!/bin/bash # Search for the definition of the unicodeNormalize function in the source/textUtils.py file. ast-grep --lang python --pattern $'def unicodeNormalize($_):\n $$$' source/textUtils.py # Locate the getPropertiesSpeech function or method in the repository. rg 'def getPropertiesSpeech' --glob '*.py' ``` Length of output: 198 --- --- Script: ```shell #!/bin/bash # Display the implementation of the unicodeNormalize function in source/textUtils.py. sed -n '/def unicodeNormalize/,/^$/p' source/textUtils.py # Display the implementation of the getPropertiesSpeech function in source/speech/speech.py. sed -n '/def getPropertiesSpeech/,/^$/p' source/speech/speech.py ``` Length of output: 47901
Actions Performed Review triggered. > Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
coderabbitai[bot] commented 1 month ago

Walkthrough

The recent changes introduce Unicode normalization to NVDA's speech and braille output, enhancing the handling of complex characters like ligatures and composed characters. This feature can be enabled via the settings dialog for both speech and braille, ensuring compatibility with various synthesizers and braille tables.

Changes

File Path Change Summary
source/braille.py Added Unicode normalization handling for braille translation, including offset processing.
source/config/configSpec.py Introduced a unicodeNormalization feature flag for different sections, including speech and braille.
source/gui/settingsDialogs.py Added unicodeNormalizationCombo control for managing Unicode normalization settings in the GUI.
source/speech/speech.py Integrated Unicode normalization in speech processing based on configuration settings.
source/textUtils.py Added utilities for Unicode normalization, including UnicodeNormalizationOffsetConverter class.
tests/unit/test_textUtils.py Added tests for UnicodeNormalizationOffsetConverter to verify normalization scenarios.
user_docs/en/changes.md Documented the addition of Unicode normalization support for speech and braille.
user_docs/en/userGuide.md Added sections explaining Unicode normalization settings for speech and braille.

Assessment against linked issues

Objective Addressed Explanation
Add optional Unicode normalization before passing strings to speech or braille (#16466) ✅
Ensure normalization handles ligatures and composed characters correctly (#16466) ✅
Provide settings to enable/disable Unicode normalization for both speech and braille (#16466) ✅

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
LeonarddeR commented 1 month ago

It looks mainly as noise to me honestly

burmancomp commented 3 weeks ago

I am getting empty braille line when reading given message in thunderbird. I encountered likely problem earlier as well with squirrelmail (web mail).

This happens when braille is tethered to review and unicode normalization is enabled for braille.

Here are log lines (running from source with current main branch code):

Input: kb(laptop):downArrow DEBUGWARNING - NVDAObjects.IAccessible.normalizeIA2TextFormatField (15:11:47.970) - MainThread (13568): Unsupported value for text-align attribute: "start" IO - speech.speech.speak (15:11:47.971) - MainThread (13568): Speaking ['removed original text\xa0\n'] DEBUGWARNING - NVDAObjects.IAccessible.normalizeIA2TextFormatField (15:11:47.992) - MainThread (13568): Unsupported value for text-align attribute: "start" DEBUGWARNING - braille.BrailleHandler._handlePendingUpdate (15:11:47.992) - MainThread (13568): Region update failed for ReviewCursorManagerRegion ('removed original text\xa0 '), object probably died Traceback (most recent call last): File "braille.py", line 2621, in _handlePendingUpdate region.update() File "braille.py", line 1378, in update super(TextInfoRegion, self).update() File "braille.py", line 503, in update converter = UnicodeNormalizationOffsetConverter(self.rawText) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "textUtils.py", line 439, in init self.computedStrToEncodedOffsets, self.computedEncodedToStrOffsets = self._calculateOffsets() ^^^^^^^^^^^^^^^^^^^^^^^^ File "textUtils.py", line 496, in _calculateOffsets originMultiplier = min(originPartLen / normalizedPartLen, 1)


ZeroDivisionError: division by zero
IO - braille.BrailleBuffer.update (15:11:47.993) - MainThread (13568):
Braille regions text: ['removed original text\xa0 ']
IO - braille.BrailleHandler.update (15:11:47.993) - MainThread (13568):
Braille window dots: -
LeonarddeR commented 3 weeks ago

Thanks for reporting. Easily reproducible on the python console: textUtils.UnicodeNormalizationOffsetConverter("removed original text\xa0 ") I will handle this as part of #16584

ABuffEr commented 3 weeks ago

Hi, viewing a FB post starting with styled chars, I noticed that requesting window title to NVDA I get the not normalized string. Anyway, the post is correctly readable by voice, that is a great, great enhancement!

LeonarddeR commented 3 weeks ago

We only normalize objects and TextInfo, not messages. This can be changed if we agree that moving by character should report normalized characters instead, because then it is much easier to apply normalization over every speech sequence, including character by character navigation. However I'm still reluctant to do so.

XLTechie commented 3 weeks ago

@LeonarddeR Can you edit the PR description to add that it closes #4631 and #11570, in case this gets reverted?

LeonarddeR commented 3 weeks ago

These are not yet closed. #16584 will probably close them.