Fix getting offsets from points in edit controls when offset numbers exceed 16 bits #8397
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LeonarddeR
force-pushed
the
editOffsetFromPoint
branch
from
eb90ec7
to
f83f193
Compare
dkager
reviewed
Jun 15, 2018
source/NVDAObjects/window/edit.py
Outdated
| lineStart=watchdog.cancellableSendMessage(self.obj.windowHandle,winUser.EM_LINEINDEX,lineNum,0) | ||
| lineStartLW = winUser.LOWORD(lineStart) | ||
| if lineStartLW > offset: | ||
| offset+= 0x10000 |
source/NVDAObjects/window/edit.py
Outdated
| # Offsets are 16 bits, therefore for large documents, we need to make sure that the correct offset is returned. | ||
| # We can calculate this by using the line number. | ||
| lineStart=watchdog.cancellableSendMessage(self.obj.windowHandle,winUser.EM_LINEINDEX,lineNum,0) | ||
| lineStartLW = winUser.LOWORD(lineStart) |
There was a problem hiding this comment.
What is contained in the LOWORD? Could the variable name make this clear?
michaelDCurran
requested changes
Jun 18, 2018
source/NVDAObjects/window/edit.py
Outdated
| # Get the last 16 bits of the line number | ||
| lineStart16=lineStart&0xFFFF | ||
| if lineStart16 > offset: | ||
| offset+=0x10000 |
There was a problem hiding this comment.
Can you better explain what these two lines do?
michaelDCurran
previously approved these changes
Jun 18, 2018
michaelDCurran
approved these changes
Jul 18, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
None
Summary of the issue:
When getting an offset from a point in edit controls, such as notepad, a 32 bit value is returned which contains the offset in the LOWORD and the line number in the HIWORD. UNtil now, NVDA was only using the LOWORD value, which caused issues when the associated offset was higher than 65535 or 0xFFFF.
Description of how this pull request fixes the issue:
When the length of the text control exceeds 0xffff, we can't simply return the LOWORD of the EM_CHARFROMPOS result, as that might be wrong. Instead, we get the start offset that is associated with the line number and calculate the real offset for the given point based on that start offset.
Testing performed:
Tested getOffsetFromPoint behavior in Notepad. I also tested Wordpad with the text info forced to EditTextInfo. That code shouldn't have been touched, and it indeed returned the correct result as well.
Furthermore, I covered the case where for example the line start offset would be <= 0xffff whereas the offset belonging to the point ought to be greater than 0xffff.
Change log entry: