Skip to content

Fix-up of #12432: Fix key error when reading appointment with categories#12435

Merged
feerrenrut merged 2 commits into
nvaccess:betafrom
CyrilleB79:fixOutlookCategories
May 20, 2021
Merged

Fix-up of #12432: Fix key error when reading appointment with categories#12435
feerrenrut merged 2 commits into
nvaccess:betafrom
CyrilleB79:fixOutlookCategories

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented May 19, 2021

Copy link
Copy Markdown
Contributor

Link to issue number:

Fix-up of #12432

Summary of the issue:

In Outlook Calendar, KeyError in the log when tabbing on an appointment having an associated category.

In #12432, .format was used without key word argument, whereas the string to be formatted contained a named interpolated value.

Details

To reproduce / test:

  • Open Outlook
  • Press control+2 to go to calendar view
  • Tab or shift+tab to focus an appointment

Note: using up/down arrows will read the appointment, but does not focus it and will not read the category.

Tested in Outlook 2016 and 365

  • with Calendar view 'working week' and day.

If you do not have an appointment with a category, you can add one the following way:

  • Open Outlook
  • Press control+2 to go to calendar view (working week for me)
  • Tab to an appointment
  • Press context menu key
  • Select "categorize" item
  • Select a category in the sub-menu

Description of how this pull request fixes the issue:

Call .format with the keyword.

Testing strategy:

Tested in Outlook that appointments with no category, one category or wo categories are correctly reported.

Known issues with pull request:

None

Change log entries:

None; fixing a not released PR.

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

Cc @feerrenrut @seanbudd

@CyrilleB79 CyrilleB79 requested a review from a team as a code owner May 19, 2021 08:57
@CyrilleB79 CyrilleB79 requested review from seanbudd and removed request for a team May 19, 2021 08:57
@feerrenrut

Copy link
Copy Markdown
Contributor

I can't reproduce this, @CyrilleB79 can you paste the log message?. It looks like I must have misunderstood how to trigger the initial category reporting. I can report categories on an email, but not on a calendar event.

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

Here is the log:

IO - inputCore.InputManager.executeGesture (11:41:41.864) - winInputHook (16236):
Input: kb(desktop):tab
ERROR - eventHandler.executeEvent (11:41:41.974) - MainThread (20248):
error executing event: gainFocus on <appModules.outlook.CalendarView object at 0x12A77970> with extra args of {}
Traceback (most recent call last):
  File "eventHandler.py", line 246, in executeEvent
    _EventExecuter(eventName,obj,kwargs)
  File "eventHandler.py", line 96, in __init__
    self.next()
  File "eventHandler.py", line 105, in next
    return func(*args, **self.kwargs)
  File "NVDAObjects\__init__.py", line 1137, in event_gainFocus
    self.reportFocus()
  File "appModules\outlook.py", line 387, in reportFocus
    categoriesText = self._generateCategoriesText(p)
  File "appModules\outlook.py", line 361, in _generateCategoriesText
    return categoriesText.format(categories)
KeyError: 'categories'

Note that reporting categories on message was already supported in NVDA 2020.4. The new feature added in #11598 with the use of ngettext was adding category reporting on appointments. You can add a category to an appointment by context menu on the appointment, the same way as you do for messages.
Anyway, that explains why your manual tests in #12432 were OK.

@CyrilleB79 CyrilleB79 closed this May 19, 2021
@CyrilleB79 CyrilleB79 reopened this May 19, 2021
@feerrenrut

Copy link
Copy Markdown
Contributor

The code here is fine, and it is obviously a bug. But given that I didn't manually test this effectively, it would be great to have proper steps on one of these PRs or issues. Currently none do.

I still can't get reproduce the error. I'm using Office 365.

  • Press ctrl+tab until the "navigation bar grouping" is announced
  • Use left / right arrows to find the calendar view button and press enter
  • Down arrow until I find an appointment (which I know has a category applied)
  • Press NVDA+tab
    Speech from speech viewer:
1:30 PM to 2:00 PM
Has appointment 2:00 PM to 2:30 PM
, , 2:00pm to 2:30pm Thursday, 20 May 2021, 1 event, 1 Busy  cell  Calendar Day View  focused  selected  Calendar Day View

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm going to go ahead with this merge, we can update the description once we understand what we are doing differently.

@feerrenrut feerrenrut merged commit d96487b into nvaccess:beta May 20, 2021
@nvaccessAuto nvaccessAuto added this to the 2021.2 milestone May 20, 2021
@feerrenrut feerrenrut modified the milestones: 2021.2, 2021.1 May 20, 2021
@CyrilleB79

Copy link
Copy Markdown
Contributor Author

@feerrenrut here are the requested clarifications.

Here is the STR that lead to the error that I have pasted above:

  • Open Outlook
  • Press control+2 to go to calendar view
  • Tab or shift+tab to an appointment in the current week that has an associated category (or more than one)
    I am using Outlook 2016 and the calendar view is 'working week'.

If you do not have an appointment with a category, you can add one the following way:

  • Open Outlook
  • Press control+2 to go to calendar view (working week for me)
  • Tab to an appointment
  • Press context menu key
  • Select "categorize" item
  • Select a category in the sub-menu

Note that with the STR you indicate, I can see 2 differences:

  • control+tab does not get me to the navigation bar group; i have to press shift+F6. Would it be a difference between 2016 and 365?
  • more important, if I use arrow keys in calendar instead of tab/shift+tab, the appointments are reported but not selected and the corresponding category is not reported, neither before this PR, nor after.

I hope that this clarifies things. Let me know if something is still unclear.

@feerrenrut

Copy link
Copy Markdown
Contributor

Yes, that helps. I had to tab to the appointment, then reading categories works as expected.

@feerrenrut

Copy link
Copy Markdown
Contributor

I have added this to the PR description.

@feerrenrut

Copy link
Copy Markdown
Contributor

@CyrilleB79 This usage will be removed for now, we believe it is causing trouble for the po file checker in the translation system. We can revisit this after the release of 2021.1

@zstanecic

zstanecic commented May 21, 2021 via email

Copy link
Copy Markdown
Contributor

@feerrenrut

feerrenrut commented May 21, 2021

Copy link
Copy Markdown
Contributor

I understand, this is not ideal. This matches the behavior of the of many similar plural strings in NVDA. We will look at re-introducing this for the next release.

@CyrilleB79 CyrilleB79 deleted the fixOutlookCategories branch May 26, 2021 08:33
seanbudd pushed a commit that referenced this pull request Jun 19, 2023
This PR is the first step to reintroduce the distinction of singulare/plural forms when formatting UI strings. The project is as follow:
1. This PR introduce three strings to be able to test the feature.
2. 2023.2 beta phase should be used to validate more widely the feature with translators
3. A second PR during 2023.3 dev cycle will implement the plural forms for all UI strings containing only one value to format them.
4. A third PR should implement more complex cases of string formatted by two or more values, e.g. such as "table with x rows and y columns".

This step-by-step approach should allow to lose less work in case an issue occurs and requires to revert changes.

### Link to issue number:
First step to implement #12445.
Restoring #11598, #12432 and #12435 that were reverted in #12448.
Unblocked by nvaccess/mrconfig#97.
### Summary of the issue:
Some UI messages or strings are reported with plural form, no matter the number passed as parameter to format them, e.g. NVDA reports "list with 1 items" with "s" even if there is only one item.

### Description of user facing changes
The following strings will be reported using singular or plural form depending on the number used to format them:
* "with %s items" (used to describe the number of items in a list on the web)
* ".1f lines" (used when reporting multiple line spacing formatting in MS Word)
* ""categories {categories}" used to report categories of appointments in MS Outlook

Translators will be able to translate singular or plural forms.

### Description of development approach
* First revert #12448 to restore the first attempt that was made to introduce `ngettext`.
* Develop a custom `npgettext` function the same way as `pgettext` was introduced in NVDA since `npgettext` is not available in Python 3.7; `pgettext` and `npgettext` are available natively in Python 3.8.
* `ngettext` and `npgettext` are made accessible without importing them; `pgettext` was already added previously in builtins.
* Implement the plural form for two strings that are more commonly used than Outlook appointments category reporting:
  * "with %s items", using `ngettext`
  * "%.1f lines", using `npgettext`
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.

4 participants