Skip to content

Fix regression when reading and deleting text in MS Excel edit fields.#9063

Merged
michaelDCurran merged 8 commits into
nvaccess:masterfrom
lukaszgo1:excelEditFields
Jan 14, 2019
Merged

Fix regression when reading and deleting text in MS Excel edit fields.#9063
michaelDCurran merged 8 commits into
nvaccess:masterfrom
lukaszgo1:excelEditFields

Conversation

@lukaszgo1

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #9042

Summary of the issue:

When #8165 was introduced it was assumed, that if particular edit field exposes line information it do not have to be accessed via displayModel. It unfortunately broke accessibility of most edit fields in Excel.

Description of how this pull request fixes the issue:

For the class used by these fields displayModel is used again.

Testing performed:

Tested with go to, find and custom list editing in Microsoft Excel 2010 and 2019.

Known issues with pull request:

I am pretty new to this part of the code, so it is possible, that it should be solved in a completely different way. @LeonarddeR your opinion would be appreciated.

Change log entry:

Section: Bug fixes

NVDA will again track cursor and announce deleted characters in edit fields in Microsoft Excel such as go to or find.

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should do either one of the following.

  1. Create a list of badUdentifiedEditClassNames and list this window class in there.
  2. If this window class is specific to Excel, which I think it is, this should be handled in an Excel appModule
  3. Similar to the windowText and windowTextLineCount properties, create a windowTextLength property that fetches the window text length:
    return watchdog.cancellableSendMessage(nav.windowHandle,winUser.WM_GETTEXTLENGTH,0,0)
    Then, only set UnidentifiedEdit as an overlay if there is text in the control. For Excel, this returns 0 even though there is text. A drawback of this approach is that unidentified text edits will always initialize with a display model overlay class when they are found to be empty.

This is clearly a bad implementation in Excel, as the text is perfectly provided using the window text.

@michaelDCurran: Do you have thoughts about this? I prefer either 2 or 3.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

For me option two looked most reasonable, so I've committed required changes. Of course if @michaelDCurran would have a different opinion I am ready to reorganize it.

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is the way to go indeed.

Comment thread source/appModules/excel.py Outdated

def chooseNVDAObjectOverlayClasses(self, obj, clsList):
windowClass = obj.windowClassName
# #9042 Edit fields in Excel have to be accessed via displayModel.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There usually is a colon after the issue number in such comments.

Suggested change
# #9042 Edit fields in Excel have to be accessed via displayModel.
# #9042: Edit fields in Excel have to be accessed via displayModel.

windowClass = obj.windowClassName
# #9042 Edit fields in Excel have to be accessed via displayModel.
if windowClass == "EDTBX":
clsList.insert(0, DisplayModelEditableText)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You will also have to import UnidentifiedEdit from NVDAObjects.window.edit and remove that from the clsList. Make sure to wrap this in a try/except statement. See the putty module as an example.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

@LeonarddeR All your commends are now addressed.

@michaelDCurran michaelDCurran merged commit 5d32b42 into nvaccess:master Jan 14, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.1 milestone Jan 14, 2019
@lukaszgo1 lukaszgo1 deleted the excelEditFields branch February 4, 2019 14:58
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.

NVDA fails to announce cursor movement and character deletion in go to edit field in excel 2010

4 participants