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

Vocalize word formatting shortcuts #10283

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

CyrilleB79
Copy link
Collaborator

@CyrilleB79 CyrilleB79 commented Sep 26, 2019

Link to issue number:

Fixes #10271

Summary of the issue:

User wants to have font formatting state announced when native font formatting shortcut are executed under Word and Outlook.

Description of how this pull request fixes the issue:

General description

This PR allows to announce the new state of font formatting when the following shortcuts are executed in MS Word (English version):

  • control+shift+D: Underline double
  • control+shift+W: Underline words only
  • control+shift+A: All caps
  • Ctrl+Shift+K: Small caps

Moreover it announce the result of the shift+F3 shortcut that change character case.

At last it include control+plus and control+shift+plus that are alternative shortcuts for subscript and superscript.

Notes about various capitalization shortcuts

  • In Word, the Shift+F3 and Ctrl+Shift+A shortcuts are slightly different:

    • Shift+F3 only modifies the case of the current selection (or the word under the cursor if there is no selection)
    • Ctrl+Shift+A also modifies the case of selected text (or current word), but in addition, it modifies the case of the characters that will be typed at insertion point of the cursor.
  • Outlook's Ctrl+Shift+A shortcut is equivalent to Word's Shift+F3, whereas Word's Ctrl+Shift+A has no equivalent under Outlook as far as I know.

Messages

Word's shortcuts Ctrl+Shift+A, Ctrl+Shift+K and Shift+F3 modify all the selection's (or current word's) capitalization. However, Ctrl+Shift+A and Ctrl+Shift+K also impact the next characters that will be typed. Thus for this shortcut "on" and "off" have been used in their announced message in order to denote a state change. On the contrary Shift+F3 announces only the result of a modification of current selection (or word); thus "on" and "off" have not been used in its message.

Testing performed:

Tested all combinations:

  • In Word and in Outlook message (edition), English version

  • For all previously listed shortcuts

  • With the following selections:

    • No selection, cursor in the middle of a word
    • No selection, cursor among case insensitive characters (numbers)
    • One single letter selected
    • Two letters selected
    • One word selected
    • Many words selected, without punctuation in middle of them
    • Sentence (many words followed by period) selected

Regarding Shift+F3, I am using a Microsoft enumeration containing also values such as wdHalfWidth, wdFullWidth, wdKatakana and wdHiragana. Seems all concepts of far-east languages.
Is there a notion equivalent to case changing in these languages? Do Shift+F3 or other shortcuts unknown to me have an impact on far-east writing?
Maybe @josephsl and @nishimotz could comment and test.

Known issues with pull request:

Summary by CodeRabbit

  • New Features

    • Enhanced vocalization of font formatting shortcuts in Word and Outlook, including underline styles and character case types.
  • Bug Fixes

    • Added specific keyboard shortcuts handling for Outlook and Word to improve user interaction.

@AppVeyorBot
Copy link

PR introduces Flake8 errors 😲

See test results for Failed build of commit 2f1a97d5a9

@AppVeyorBot
Copy link

PR introduces Flake8 errors 😲

See test results for Failed build of commit 554b3f3958

Ctrl+U Toggle single underline
Ctrl+Shift+D Toggle double underline
Ctrl+Shift+W: Toggle word only underline
Ctrl+shift+A: Toggle between All cap and No cap
Ctrl+shift+K: Toggle between Small cap and No cap.
Shift+F3: Toggle between Upper case, lower case and Capitalize first letter.
@bdorer
Copy link
Sponsor

bdorer commented Jun 21, 2020

is there any progress here?

@CyrilleB79
Copy link
Collaborator Author

On my side, the PR is ready.
Feel free to test it (maybe the gesture.ini file must be updated according to your locale) and provide feedback if you want.
This PR has been labelled as feature by @feerrenrut. Maybe such PRs are not prioritarised these days by NVAccess. Let's see what is happening when #11006 is resolved.

@CyrilleB79
Copy link
Collaborator Author

I am adding two additional shortcut keys for toggleSuperscriptSubscript script:

  • kb:control+plus
  • kb:control+shift+plus
    Indeed, they are also valid shortcuts key to format the text to superscript or subscript in English version of Word or Outlook, as well as in some other versions (e.g. French, Italian). These shortcuts cannot be executed with English (US) keyboard, but can be executed in English Word with another keyboard allowing to type "+" without any modifier, such as German or Italian.

I will update the initial description of this PR accordingly.

@AppVeyorBot
Copy link

See test results for failed build of commit 7c017fd5ac

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.

What about if the users customizes keyboard shortcuts in word?

source/NVDAObjects/window/winword.py Show resolved Hide resolved
Comment on lines 1447 to 1449
# Under Outlook, calling the script quickly a second time gives strange results.
# Case value passes sometimes through an intermediate value.
# So poll the value a second time.
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be that the first operation hasn't yet completed. It would probably be best if nvda scripts queued subsequent runs until the first is complete. Could you see if you can confirm if that is happening here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do you perform this? I.e. how do you delay the second script until the end of the first?

Moreover, I have tried to reproduce the issue with only one polling, but I cannot anymore. I will try on another version of Word later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have just tested on another Word and Outlook version and I cannot reproduce the issue either. Both Office are 2016, but one of them was not up-to-date.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do you perform this? I.e. how do you delay the second script until the end of the first?

We'd need to first confirm that is what is happening, my suggestion was really just a guess. Since I didn't know, I have done some investigation, the short version is that it seems unlikely that the script is being called again before it completes. The long answer follows:

To work it out we can look at how the gestures are called.

It all starts with the hookThreadFunc in source/winInputHook.py. This runs in it's own thread, it registers a Low level hook function for the keyboard. Unfortunately this is where the questions start:

A global hook procedure can be called in the context of any application in the same desktop as the calling thread, so the procedure must be in a separate DLL module.

  • Can this callback be re-entered by the Windows system?
    • I couldn't find any information about this.

Another interesting piece of advice from the docs for "LowLevelKeyboardProc callback function" which we are apparently ignoring is:

Note Debug hooks cannot track this type of low level keyboard hooks. If the application must use low level hooks, it should run the hooks on a dedicated thread that passes the work off to a worker thread and then immediately returns. In most cases where the application needs to use low level hooks, it should monitor raw input instead. This is because raw input can asynchronously monitor mouse and keyboard messages that are targeted for other threads more effectively than low level hooks can. For more information on raw input, see Raw Input.

Ok, well I say "ignoring", but it looks like most paths result in adding an event to the event queue. The logic is complicated enough that it is hard to confirm this happens for all paths. If we assume it does happen for all paths, then I think we can be fairly confident that the script can't be called until it is exited.

I'm going to call on @michaelDCurran to take a look at this to see if there is anything I might have missed here, or if he can clarify anything further.

Copy link
Collaborator Author

@CyrilleB79 CyrilleB79 Feb 5, 2021

Choose a reason for hiding this comment

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

@feerrenrut thanks for your detailed explanations (and also for the summary!).

I have tried again but I cannot reproduce the issue anymore, This PR is quite old (it was one of my first PRs) and I do not remember exactly which type of stress test may have produced the issue; I also do not remember for example if I had tried to restart my computer.
I have thus removed this double polling. If an issue occur we my investigate on this new basis.

However, I keep this conversation open expecting @michaelDCurran's point of view.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@feerrenrut, have you had the opportunity to discuss with @michaelDCurran on the double script call potential issue.
Since I have removed the double-polling anyway, this conversation is not blocked anymore IMO. Do you agree?

@CyrilleB79
Copy link
Collaborator Author

@feerrenrut wrote:

What about if the users customizes keyboard shortcuts in word?

This is not supported in this PR. This PR uses more or less the same strategy than what already exists in NVDA to report text formatting shortcuts in Word such as italic, underline and bold. Thus it suffers from the same limitations. If the user changes the default shortcuts in Word, the result of the new shortcut will not be reported.
I do not know if many users change default shortcuts.
If the user is a dev / tech-friendly people, he may modify manually his gestures.ini file, as translators do. But in any case, it is not an operation that any user can execute.

@feerrenrut
Copy link
Contributor

This is not supported in this PR. This PR uses more or less the same strategy than what already exists in NVDA to report text formatting shortcuts in Word such as italic, underline and bold. Thus it suffers from the same limitations.

Ok, fair point this is not a new concern.

@lukaszgo1
Copy link
Contributor

@feerrenrut @michaelDCurran Would it be possible to move this PR forward?

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.

Overall the code looks good.

But testing this locally looking speech viewer:

  • Pressing ctrl+u shows two lines :
    Underline Single
    Underline On
    
  • Pressing ctrl+b shows two lines, note the difference in case:
    Bold on
    Bold On
    

Only the second sequence is heard, meaning that some information is lost in the underline case.
I haven't tested the other shortcuts yet, but I think this should be addressed.

@josephsl
Copy link
Collaborator

josephsl commented Jul 8, 2021 via email

@CyrilleB79
Copy link
Collaborator Author

Regarding duplicate format reporting, the issue is not specific to this PR, as explained by @josephsl.

Looking again at this old PR, I am not so happy with the announcement "Underline single" instead of today's "Underline on". Maybe in the case of much more common single underline we can just keep "Underline on".
What do you think?

@feerrenrut
Copy link
Contributor

Regarding duplicate format reporting, the issue is not specific to this PR,

Even though it is not specific to this PR, it is a conflict in the UI. The new messages may not be heard, it is also causing NVDA to do more than it needs to.

Looking again at this old PR, I am not so happy with the announcement "Underline single" instead of today's "Underline on". Maybe in the case of much more common single underline we can just keep "Underline on".
What do you think?

I'm not sure, I assume that the other values might be "underline double", any others? This provides more information than just "underline on".

@CyrilleB79
Copy link
Collaborator Author

Regarding duplicate format reporting, the issue is not specific to this PR,

Even though it is not specific to this PR, it is a conflict in the UI. The new messages may not be heard, it is also causing NVDA to do more than it needs to.

I may have not been clear enough in my previous comment. Let me give some more details:

  1. I have understood that there is already a PR from @josephsl to fix the double reporting issue: Word 365: do not announce font attribute toggle messages if raised from UIA notification event #10990. @josephsl can you confirm this? If yes, there is no point in fixing the issue in this PR.
  2. In my version of office, the UIA notifications are raised only only upon the following key press and only in MS Word (not Outlook): ctrl+I (italic), ctrl+U (underline) and ctrl+G (French equivalent of ctrl+B = bold). There is no UIA notification for the various capitalization shortcuts, the other underline shortcuts (ctrl+shift+D = "double" and ctrl+shift+W = "words only"). There is also no UIA notification for the following alternative shortcuts: ctrl+shift+I/U/B, ctrl+plus and ctrl+shift+plus.
    To summarize, none of the new shortcut key introduced in this PR have the double reporting issue in my version of Office.
    @joseph, maybe however the set of shortcuts that report UIA notification has been extended in newer versions of MS Office. Could you report the result withe all the shortcuts listed in this PR's description? Thanks.

So IMO, this PR can be progressed independantly from #10990.
Anyway, if you have another opinion, I would prefer to attribute a blocked label to this PR, until #10990 is merged.

@CyrilleB79
Copy link
Collaborator Author

Looking again at this old PR, I am not so happy with the announcement "Underline single" instead of today's "Underline on". Maybe in the case of much more common single underline we can just keep "Underline on".
What do you think?

I'm not sure, I assume that the other values might be "underline double", any others? This provides more information than just "underline on".

OK I let you the final decision on this point. So unless there is another comment regarding this topic, I keep the announcement "Underline single".
The other underline styles that have a Word shortcut key are: underline double (control+shif+D) and underline words only (control+shift+W). But I guess they are much less commonly used.

@feerrenrut
Copy link
Contributor

It seems like there are some complexities here. Someone will need to deep dive on this to

  • enumerate the different situations, whether UIA and object model are available.
  • Are UIA notifications translated?

In situations where both object model and UIA notifications are available, a setting to select between them would be a safe fallback. In other situations the appModule should select what type of notification system is used. This may need to be at individual event granularity.

Given the clash here, I don't think this can be merged until we have these answers. @CyrilleB79 I think we should convert this PR back to draft. Perhaps you and @josephsl can work together on reaching a solution for this?

@feerrenrut feerrenrut marked this pull request as draft July 13, 2021 11:15
@CyrilleB79
Copy link
Collaborator Author

@feerrenrut I do not understand if you disagree with or if you just missed my proposal to handle the two points in separate PRs:

  • Support more font formatting shortcut reporting via object model (as done today); this is the object of the current PR
  • Deal with the issue of potential double reporting, i.e. choose when to use object model and when to use UIA: should be implemented in a new PR

IMO it's more clear that these two tasks be handled separately, i.e. in 2 PRs.

And I think they do not depend so much on each other. Anyway if you disagree on this last point, we may set this PR on hold while the other one (UIA vs object model) is implemented and merged. Just let me know before I continue on these topics.

@feerrenrut
Copy link
Contributor

When we are in doubt about a PR we opt towards no change. There are several open questions here, the resulting user experience of this change can't be determined until they are answered. I'm concerned that it isn't certain which message a user will receive (UIA or object model).

I would prefer to hold off on merging this PR until these questions are answered or resolved. I'm happy for that to happen via another PR. Until then I'll mark this blocked.

@CyrilleB79
Copy link
Collaborator Author

OK. Thanks for this clarification.
I will try to have a look on the other topic with Joseph (if he can) or someone else since I do not have a Word 365 or 2019 to test with the most recent versions of Office.

I'm concerned that it isn't certain which message a user will receive (UIA or object model).

Regarding this point, the situation is the following in the current version of NVDA (unchanged with this PR):

  • In the case where the UIA notifications cannot be reported (i.e. Windows version < Windows 10 1709 or older version of Word), only NVDA's notification based on the object model is reported
  • In the case UIA can be reported, the notification based on the object model begins to be reported, but is interrupted by the UIA notification. So actually, in this case, only the UIA notification can be fully heard, but a little part of the object model notification can sometimes be heard beforehand.

@seanbudd
Copy link
Member

Closing as abandoned

@seanbudd seanbudd closed this Jun 17, 2024
@seanbudd seanbudd added the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Jun 17, 2024
@CyrilleB79
Copy link
Collaborator Author

@seanbudd, this PR is not abandoned but blocked by #10990. I agree that this is quite old.

In the meantime we have been able to see the evolution of UIA notifications regarding text formatting. After all these years, Microsoft has not added new notifications, i.e. only bold, italic and underline are being reported by UIA notifications.

So to progress this PR, I propose to remove the changes related to bold, italic and underline and underline, i.e. all the shortcuts conflicting with UIA notifications, since it's the goal of #10990.

@seanbudd do you agree with this plan? If not, why?

Reopening and removing the "Abandoned" label. We may close again after discussion if needed.

@CyrilleB79 CyrilleB79 reopened this Jun 17, 2024
@seanbudd
Copy link
Member

this plan makes sense, my assumption was that the work was abandoned. thanks

@CyrilleB79 CyrilleB79 removed the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Jun 18, 2024
@seanbudd
Copy link
Member

I'm marking this as ready for review with the note that it is currently blocked

@seanbudd seanbudd marked this pull request as ready for review June 25, 2024 04:30
Copy link
Contributor

coderabbitai bot commented Jun 25, 2024

Walkthrough

The primary changes include adding new enumerations for underline styles and character case types in Microsoft Word, updating scripts to toggle formatting, and improving the narration of formatting states in NVDA when using keyboard shortcuts in Word. Also, specific keyboard shortcuts handling has been enhanced for Outlook integration.

Changes

Files Change Summaries
source/NVDAObjects/window/winword.py Added enumerations for underline styles and character case types, defined their descriptions, and updated formatting toggle scripts.
source/appModules/outlook.py Extended the script_tab method in OutlookWordDocument class to include a gestures dictionary for handling keyboard shortcuts.
user_docs/en/changes.md Documented the new feature to vocalize the result of more font formatting shortcuts in Word and Outlook.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant NVDA
    participant Word/Outlook

    User->>Word/Outlook: Press formatting shortcut (e.g., Ctrl+B)
    Word/Outlook-->>NVDA: Send formatting state change (e.g., bold on)
    NVDA-->>User: Announce formatting state change (e.g., "Bold on")
Loading

Assessment against linked issues

Objective (Issue #10271) Addressed Explanation
Precise announcing of the state of formatting settings when hotkey is pressed.

Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e57d7e5 and 36c703f.

Files selected for processing (3)
  • source/NVDAObjects/window/winword.py (10 hunks)
  • source/appModules/outlook.py (1 hunks)
  • user_docs/en/changes.md (1 hunks)
Files skipped from review due to trivial changes (1)
  • user_docs/en/changes.md
Additional context used
Path-based instructions (2)
source/appModules/outlook.py (2)

Pattern **/*: Focus on code smells, logic errors, edge cases, missing test cases, security flaws and serious issues. Avoid commenting on minor issues such as linting, formatting and style issues. This project uses tabs instead of spaces, do not suggest usage of spaces over tabs. Are there any 'red flags' in this code that might warrant closer investigation from a security standpoint? Explain what makes them suspicious.


Pattern **/*.py: _, pgettext, ngettext, and ngettext are defined globally, errors for this being undefined can be ignored.

source/NVDAObjects/window/winword.py (2)

Pattern **/*: Focus on code smells, logic errors, edge cases, missing test cases, security flaws and serious issues. Avoid commenting on minor issues such as linting, formatting and style issues. This project uses tabs instead of spaces, do not suggest usage of spaces over tabs. Are there any 'red flags' in this code that might warrant closer investigation from a security standpoint? Explain what makes them suspicious.


Pattern **/*.py: _, pgettext, ngettext, and ngettext are defined globally, errors for this being undefined can be ignored.


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
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • 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.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @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 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 for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Comment on lines +636 to +640
__gestures = {
"kb:control+shift+w": None, # Shortcut existing in Word but not in Outlook
"kb:control+shift+b": None, # Shortcut existing in Word but opens address book in Outlook
"kb:control+shift+a": "changeCase", # In Outlook Ctrl+Shift+A is equivalent to Shift+F3
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor the __gestures dictionary for clarity and maintainability.

The __gestures dictionary is used to map keyboard shortcuts to their corresponding actions. The current implementation uses None to signify shortcuts that are not applicable in Outlook, which might confuse future maintainers. Consider using a more descriptive placeholder or a comment to explain why these shortcuts are set to None.

  __gestures = {
+   # Shortcuts that are specific to Word and have different meanings in Outlook
    "kb:control+shift+w": None,  # Shortcut existing in Word but not in Outlook
    "kb:control+shift+b": None,  # Shortcut existing in Word but opens address book in Outlook
    "kb:control+shift+a": "changeCase",  # In Outlook Ctrl+Shift+A is equivalent to Shift+F3
  }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
__gestures = {
"kb:control+shift+w": None, # Shortcut existing in Word but not in Outlook
"kb:control+shift+b": None, # Shortcut existing in Word but opens address book in Outlook
"kb:control+shift+a": "changeCase", # In Outlook Ctrl+Shift+A is equivalent to Shift+F3
}
__gestures = {
# Shortcuts that are specific to Word and have different meanings in Outlook
"kb:control+shift+w": None, # Shortcut existing in Word but not in Outlook
"kb:control+shift+b": None, # Shortcut existing in Word but opens address book in Outlook
"kb:control+shift+a": "changeCase", # In Outlook Ctrl+Shift+A is equivalent to Shift+F3
}

Comment on lines +87 to +90
wdUnderlineNone = 0
wdUnderlineSingle = 1
wdUnderlineWords = 2
wdUnderlineDouble = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation suggestion for enumerations.

The enumerations for underline styles (wdUnderlineNone, wdUnderlineSingle, etc.) and their corresponding descriptions (wdUnderlineTypeDescriptions) are well-defined. However, adding a comment at the top of these enumerations explaining how and where these constants can be found or updated would improve maintainability. This aligns with the feedback from feerrenrut in the previous comments.

+ # These constants represent the underline styles used in Microsoft Word. For updates or additions, refer to the official Microsoft Office VBA documentation.

Also applies to: 354-389

Comment on lines +1551 to +1565
@script(gesture="kb:shift+f3")
def script_changeCase(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.Range.Case
)
# Translators: a message when changing case in Microsoft Word
ui.message(wdCharacterCaseTypeLabels.get(val))

Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for script_changeCase.

The method script_changeCase retrieves the case of the text and displays it, but does not handle the case where val may not be in wdCharacterCaseTypeLabels.

- ui.message(wdCharacterCaseTypeLabels.get(val))
+ case_message = wdCharacterCaseTypeLabels.get(val, _("Unknown case"))
+ ui.message(case_message)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@script(gesture="kb:shift+f3")
def script_changeCase(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.Range.Case
)
# Translators: a message when changing case in Microsoft Word
ui.message(wdCharacterCaseTypeLabels.get(val))
@script(gesture="kb:shift+f3")
def script_changeCase(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.Range.Case
)
# Translators: a message when changing case in Microsoft Word
case_message = wdCharacterCaseTypeLabels.get(val, _("Unknown case"))
ui.message(case_message)

Comment on lines +1486 to +1502
def script_toggleUnderline(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail, or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor. For now, just let the gesture through and don't erport anything.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val=self._WaitForValueChangeForAction(lambda: gesture.send(),lambda: self.WinwordSelectionObject.font.underline)
if val:
# Translators: a message when toggling formatting in Microsoft word
ui.message(_("Underline on"))
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.font.underline
)
if val != wdUnderlineNone:
msg = wdUnderlineTypeDescriptions.get(val)
if msg:
# Translators: a message when toggling formatting in Microsoft word
ui.message(_("Underline {style}").format(style=msg))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure robust handling of multiple underline styles.

The method script_toggleUnderline handles toggling of various underline styles. It correctly checks if the underline style is not wdUnderlineNone and retrieves the message from wdUnderlineTypeDescriptions. However, the method should robustly handle the case where val might not be in wdUnderlineTypeDescriptions, which could potentially lead to a KeyError.

- msg = wdUnderlineTypeDescriptions.get(val)
+ msg = wdUnderlineTypeDescriptions.get(val, _("Unknown style"))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def script_toggleUnderline(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail, or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor. For now, just let the gesture through and don't erport anything.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val=self._WaitForValueChangeForAction(lambda: gesture.send(),lambda: self.WinwordSelectionObject.font.underline)
if val:
# Translators: a message when toggling formatting in Microsoft word
ui.message(_("Underline on"))
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.font.underline
)
if val != wdUnderlineNone:
msg = wdUnderlineTypeDescriptions.get(val)
if msg:
# Translators: a message when toggling formatting in Microsoft word
ui.message(_("Underline {style}").format(style=msg))
def script_toggleUnderline(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.font.underline
)
if val != wdUnderlineNone:
msg = wdUnderlineTypeDescriptions.get(val, _("Unknown style"))
if msg:
# Translators: a message when toggling formatting in Microsoft word
ui.message(_("Underline {style}").format(style=msg))

Comment on lines +1528 to +1549
@script(gestures=["kb:control+shift+a", "kb:control+shift+k"])
def script_toggleCaps(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or it's within Windows Defender Application Guard.
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
margin = self.WinwordDocumentObject.PageSetup.LeftMargin
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.paragraphFormat.LeftIndent
lambda: (self.WinwordSelectionObject.font.allcaps, self.WinwordSelectionObject.font.smallcaps)
)
msg = self.getLocalizedMeasurementTextForPointSize(margin + val)
ui.message(msg)
if val[0]:
# Translators: a message when toggling formatting to 'all capital' in Microsoft word
ui.message(_("All caps on"))
elif val[1]:
# Translators: a message when toggling formatting to 'small capital' in Microsoft word
ui.message(_("Small caps on"))
else:
# Translators: a message when toggling formatting to 'No capital' in Microsoft word
ui.message(_("Caps off"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor repeated code in script_toggleCaps.

The method script_toggleCaps has repeated checks and messages for toggling caps. This can be refactored to reduce redundancy and improve clarity.

- if val[0]:
-     ui.message(_("All caps on"))
- elif val[1]:
-     ui.message(_("Small caps on"))
- else:
-     ui.message(_("Caps off"))
+ cap_styles = ["All caps", "Small caps", "No caps"]
+ cap_index = val.index(True) if True in val else 2  # Default to "No caps" if neither is True
+ ui.message(_("{style} on").format(style=cap_styles[cap_index]))
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@script(gestures=["kb:control+shift+a", "kb:control+shift+k"])
def script_toggleCaps(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or it's within Windows Defender Application Guard.
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
margin = self.WinwordDocumentObject.PageSetup.LeftMargin
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: self.WinwordSelectionObject.paragraphFormat.LeftIndent
lambda: (self.WinwordSelectionObject.font.allcaps, self.WinwordSelectionObject.font.smallcaps)
)
msg = self.getLocalizedMeasurementTextForPointSize(margin + val)
ui.message(msg)
if val[0]:
# Translators: a message when toggling formatting to 'all capital' in Microsoft word
ui.message(_("All caps on"))
elif val[1]:
# Translators: a message when toggling formatting to 'small capital' in Microsoft word
ui.message(_("Small caps on"))
else:
# Translators: a message when toggling formatting to 'No capital' in Microsoft word
ui.message(_("Caps off"))
@script(gestures=["kb:control+shift+a", "kb:control+shift+k"])
def script_toggleCaps(self, gesture):
if not self.WinwordSelectionObject:
# We cannot fetch the Word object model, so we therefore cannot report the format change.
# The object model may be unavailable because this is a pure UIA implementation such as Windows 10 Mail,
# or its within Windows Defender Application Guard.
# Eventually UIA will have its own way of detecting format changes at the cursor.
# For now, just let the gesture through and don't report anything.
return gesture.send()
val = self._WaitForValueChangeForAction(
lambda: gesture.send(),
lambda: (self.WinwordSelectionObject.font.allcaps, self.WinwordSelectionObject.font.smallcaps)
)
cap_styles = ["All caps", "Small caps", "No caps"]
cap_index = val.index(True) if True in val else 2 # Default to "No caps" if neither is True
ui.message(_("{style} on").format(style=cap_styles[cap_index]))

@CyrilleB79
Copy link
Collaborator Author

@seanbudd with my last changes, I do not consider this PR blocked anymore by #10990.

However, I still have to re-test all the use cases; it seems to me that not all of them are functional. That's why I kept this PR as draft. I will also address the Rabbit's review and let you know when it is ready.

Or was there any other reason why you consider this PR blocked?

@seanbudd
Copy link
Member

Thanks, good to know, it was a misunderstanding on my part

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 2, 2024
@seanbudd
Copy link
Member

seanbudd commented Sep 2, 2024

@CyrilleB79 - do you plan to do any further work here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app/microsoft-office conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

precise Announcing of the state of formatting settings when a hotkey in word is pressed
7 participants