Allow quick navigation for spelling errors in MS Word using w and shift+w#7258
Conversation
…ntly doing nothing
michaelDCurran
left a comment
There was a problem hiding this comment.
Code looks good. But:
In the past we added support for reportting gramma errors in Microsoft Word. But from the change log:
Removed reporting of grammar errors, as this causes crashes in Microsoft Word. (#5954, #5877)
It is possible that this usage out-of-process will not cause a crash, but this must be tested on Windows XP to make sure.
The safest option would be for now to only support spelling errors.
| return browseMode.mergeQuickNavItemIterators([spellingErrors,grammaticalErrors],direction) | ||
| elif nodeType=="graphic": | ||
| return GraphicWinWordCollectionQuicknavIterator(nodeType,self,direction,rangeObj,includeCurrent).iterate() | ||
| return GraphicWinWordCollectionQuicknavIterator(nodeType,self,direction,rangeObj,includeCurrent).iterate() |
There was a problem hiding this comment.
Why did this line change?
There was a problem hiding this comment.
See above, another extra space here which shouldn't exist
| return browseMode.mergeQuickNavItemIterators([comments,revisions],direction) | ||
| elif nodeType in ("table","container"): | ||
| return TableWinWordCollectionQuicknavIterator(nodeType,self,direction,rangeObj,includeCurrent).iterate() | ||
| return TableWinWordCollectionQuicknavIterator(nodeType,self,direction,rangeObj,includeCurrent).iterate() |
There was a problem hiding this comment.
Why did this line change?
There was a problem hiding this comment.
This was, probably by accident, indented with two tabs and one space, I removed the space to bring this in line with other return statements here.
What has Windows Xp to do with this, if I may ask? I don't see any references to XP in #5954
I agree, however that would mean we'd never test this properly. How about an incubation in Next and asking people (e.g. on NVDA devel) to specifically test this feature with multiple versions of Office? We'd make sure that this has been tested somewhat more thoroughly before merging. A try build might be a viable alternative. Of course I'd be happy to remove support for grammatical errors if desired, but it feels a bit like choosing de path of least resistance here, which doesn't necessarily have to be bad of course. |
|
True, the comment about XP is irrelevant, I remembered the issue
incorrectly.
However, as we do not report grammar errors currently, adding quicknav
to them would be confusing.
Therefore, until we have the time to properly research a new and safe
way to implement grammar errors, we only want to take the PR if the
grammar support is removed.
|
|
No problem.
Do you want me to remove the
GrammaticalErrorWinWordCollectionQuicknavIterator and
WordDocumentGrammaticalErrorQuickNavItem classes altogether, or should I
add a comment to them saying that they are currently not in use?
|
|
Please remove it all for now. Obviously branches can be kept around if
we want to use the code at a later date.
|
…ext of the spelling error in a similar way to how this is done for comments and revisions
|
I think this approach is a bit nicer than the one I used before with super, but if you disagree, I'm happy to roll this back. |
… comments in future. 1. Translators reported that some new translatable strings introduced in PR #7258 were missing translator comments. Add translator comments for these. 2. Add some code (`checkPot`) to check for missing translator comments in the translation template (pot). For now, we have a set of expected failures, since fixing all existing 87 messages that don't have translator comments is going to take some work. However, we want to fail for any new messages that get introduced without comments. 3. Run `checkPot` as part of `scons tests`. This means tests (and thus builds) will fail so we learn about these problems early. 4. `checkPot` can also be run alone with `scons checkPot`. 5. Now that `scons tests` runs `checkPot` (which depends on `pot`), don't explicitly run `scons pot` on AppVeyor.
… comments in future. 1. Translators reported that some new translatable strings introduced in PR #7258 were missing translator comments. Add translator comments for these. 2. Add some code (`checkPot`) to check for missing translator comments in the translation template (pot). For now, we have a set of expected failures, since fixing all existing 87 messages that don't have translator comments is going to take some work. However, we want to fail for any new messages that get introduced without comments. 3. Run `checkPot` as part of `scons tests`. This means tests (and thus builds) will fail so we learn about these problems early. 4. `checkPot` can also be run alone with `scons checkPot`. 5. Now that `scons tests` runs `checkPot` (which depends on `pot`), don't explicitly run `scons pot` on AppVeyor.
… comments in future. 1. Translators reported that some new translatable strings introduced in PR #7258 were missing translator comments. Add translator comments for these. 2. Add some code (`checkPot`) to check for missing translator comments in the translation template (pot). For now, we have a set of expected failures, since fixing all existing 87 messages that don't have translator comments is going to take some work. However, we want to fail for any new messages that get introduced without comments. 3. Run `checkPot` as part of `scons tests`. This means tests (and thus builds) will fail so we learn about these problems early. 4. `checkPot` can also be run alone with `scons checkPot`. 5. Now that `scons tests` runs `checkPot` (which depends on `pot`), don't explicitly run `scons pot` on AppVeyor.
|
The "errors" radio button in the elements list in Word is missing a capital "E". Seems worth fixing for cosmetic reasons, but not worth a PR. |
Link to issue number:
#6942
Summary of the issue:
Currently, there is no effective way in MS Word to list and navigate spelling errors. There is the alt+f7 key stroke, but activating that automatically pop ups the context menu to change the particular error.
Description of how this pull request fixes the issue:
This pull request adds a new quick navigation command for errors, bound to the letter w. Consult the discussion in #6942 for more details about how we made this decision.
The error command navigates to spelling and grammatical errors in Word. Also, errors have been added to the Word elements list and can be listed there.
Testing performed:
Purposely made some grammar and spelling mistakes. I was able to jump through those with w (forwards) and shift+w (backwards). Also, I was able to list them in the elements list. Spelling errors which are part of grammatical errors are shown at a sub level in the elements list tree, as expected.
Known issues with pull request:
Spelling and grammar checking while typing should have been enabled in Word for this feature to work correctly. Spelling error checking is enabled by default, for grammar, this doesn't seem to be the case. should we document this?
Change log entry: