Style navigation QuickNav command #16050
Conversation
See test results for failed build of commit 3297106adb |
|
The bot says some system test with tag Installer is failing. I couldn't find any information on waht test is failing. Not sure if this is flaky test. If it's not flaky - could someone help me to find which test is failing? It wasn't very clear to me based on bot's comment. |
|
Yes I think that test failure is unrelated |
seanbudd
added
the
conceptApproved
|
In an Outlook message, I was hoping to jump to "answers in red" sent by a colleague of mine. Unfortunately, it does not work. I get errors such as the following when calling all 4 commands (here with Move to next different style text command): |
|
Regarding unassigned keys: Maybe we should merge this PR and #16031 with unassigned gestures and test them during alpha stage. Then at the end of alpha stage, we can figure if one or both of these feature need to have an assigned gesture and add them. |
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Łukasz Golonka <lukasz.golonka@mailbox.org>
See test results for failed build of commit d32acb6519 |
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
|
@mltony the feature does not seem to work with Word legacy; I did not re-test with UIA. Could you double-check that all command types work in Word with and without UIA? Let me know when you have re-tested this successfully. Thanks. |
|
@CyrilleB79 - as I mentioned #16171 and #16172 are word specific problems that prevent style navigation to work correctly in MSWord. Both issues affect both legacy and UIA word support. Until both are fixed, style navigation won't work correctly in MSWord. |
|
@mltony, sorry to come a bit late with it, but I had not paid attention to the following known issue:
My personal opinion is that merging same style across paragraph boundaries is more than desirable. For me, the main use case of this PR is : "See my comments in red the message below". In a message with many bullet paragraphs (e.g. meeting minutes), with only few comments in red, I do not expect the jump commands to jump to each bullet. |
Re #16171: as mentioned there, word is able to read text with formatting information if NVDA is configured for this. Thus you should probably use the same technique for style navigation. Re #16172: I have not reviewed the code. But it seems strange to me that you cannot work it around, e.g. catching the exception to indicate the end of the document. Actually Word has a browse mode with a working quick nav capability. Why is style navigation impacted and other quick nav commands not? I hope you can fix #16171 and #16172 in this PR. Especially because Outlook uses Word browse mode on by default, and because the "See my comments in red in the message below" is a very common use case. Though, if you confirm that you cannot fix them, you should:
|
|
What @CyrilleB79 describes is the behavior I would expect also. I'm not sure how
this feature is useful if it doesn't act that way.
|
|
@CyrilleB79, @XLTechie, |
|
Thanks @mltony. This actually works much better in Word. There still seems to be little issues with links of e-mails and URLs. Try to type the following: Depending on where you have initially put the cursor, you get unexpected results. E.g. cursor before the link, or after its first chararacter, or in the middle, or on the penultimate character, or on the ultimate character, or just after the last character of the link, etc. |
|
@CyrilleB79 , |
This function is needed for both #8518 and #16050. Summary of the issue: Suppose we have TextInfo that represents a paragraph of text: ``` > s = paragraphInfo.text > s 'Hello, world!\r' ``` Suppose that we would like to put the cursor at the first letter of the word 'world'. That means jumping to index 7: ``` > s[7:] 'world!\r' ``` The problem is that calling paragraphInfo.move(UNIT_CHARACTER, 7, "start") is not guaranteed to achieve desired effect. There are two main reasons for that: 1. In Wide character encoding, some 4-byte unicode characters are represented as two surrogate characters, whereas in pythonic string they would be represented by a single character. 2. In non-offset TextInfos (e.g. UIATextInfo) there is no guarantee on the fact that TextInfos.move(UNIT_CHARACTER, 1)would actually move by exactly 1 character. A good illustration of this is in Microsoft Word with UIA enabled always, the first character of a bullet list item would be represented by three pythonic characters: ◦ Bullet character "•" ◦ Tab character \t ◦ And the first character of of list item per se. 3. The third problem of TextInfo.move(UNIT_CHARACTER) function is its performance in some implementations. In particular, moving by 10000 characters in Notepad++ takes over a second on a reasonably modern PC. I might not need to move by 10000 characters in my upcoming PRs, but I will need to move by a few thousands for sure since for sentence navigation I would need to move within a paragraph and some large paragraphs in typical texts can easily be few thousands characters. I need to find both beginning and end textInfos, and if each operation takes say 200ms, then we'd be wasting almost half a second on just moving by characters. Since there were previous concerns about sentence navigation being not fast enough, II would like to introduce this efficient implementation. Here is how this can be done efficiently using this PR: ``` > info = paragraphInfo.moveToPythonicOffset(7) > info.setEndPoint(paragraphInfo, "endToEnd") > info.text 'world!\r' ``` Description of development approach 1. For general case, I implemented binary-search-like algorithm. I explained it in great detail in the code. Please see def moveToPythonicOffset function in textInfos\__init__.py. 2. I provided optimized implementations for OffsetsTextInfo and CompoundTextInfo. 3. I refactored textUtils.py making it conformant to OOP style. I implemented UTF8OffsetConverter and dummy IdentityOffsetConverter as well as their abstract base class and a function getOffsetConverter that selects correct converter based on encoding. I renamed a couple of methods of WideStringOffsetConverter in order to remove the word wide - as now I would like to use similar methods for UTF8 converter, and it has nothing to do with wide strings.
|
I updated this to use moveToCodepointOffset function - this is now ready for review. I also fixed a small bug in moveToCodepointOffsets and added more unit tests to cover that bug. |
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Link to issue number:
Closes #16000
Summary of the issue:
Adding jump to same style and jump to different style QuickNav commands.
Description of user facing changes
Adding jump to same style and jump to different style QuickNav commands. They are not assigned to any keyboard gestures by default.
Description of development approach
Scanning document by paragraph in the desired direction and analyzing styles within each paragraph.
Testing strategy:
Tested manually in Chrome and Firefox.
Added a system test.
Known issues with pull request:
Another quirk to be aware of: in this PR I treat paragraphs separately. For example if we have an entire document written in the same style, then jump to next same style command would iterate over paragraphs. In theory we can try to merge paragraphs and parts of paragraphs written in the same style together, but this would make code more complicated and I personally feel current approach is more intuitive, but LMK if you think otherwise.Update: as requested, now I changed the code so that every chunk found by this pR can span multiple paragraphs.Code Review Checklist: