Skip to content

Fix error fetching details#13456

Merged
feerrenrut merged 6 commits into
betafrom
fixFirefoxDetails
Mar 15, 2022
Merged

Fix error fetching details#13456
feerrenrut merged 6 commits into
betafrom
fixFirefoxDetails

Conversation

@feerrenrut

@feerrenrut feerrenrut commented Mar 10, 2022

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #13433
Fixes #13430

Summary of the issue:

Gecko applications (Firefox / thunderbird / instantbird) occasionally are raising a COM error "No such interface supported"
when aria-details relation is fetched.
This exception should be handled as per getIA2RelationFirstTarget.
Additionally:

  • getIA2RelationFirstTarget failed to convert IUnknown to NVDAObject.IAccessible. This was missed due to failure with typing system, ctypes.POINTER(IUnknown) is not recognised and becomes Any.
  • Unecessary firefox error loging on a missing detailsSummary.

Description of how this pull request fixes the issue:

  • Prevent ComError escaping when checking for aria-details by using getIA2RelationFirstTarget directly.
  • Ensure that IUnkown is converted to an IAccessible in getIA2RelationFirstTarget
  • Remove unecessary firefox error log on missing detailsSummary.

Testing strategy:

Using https://codepen.io/reefturner/pen/RwjBXPv
Test reporting details in firefox and in Edge.

Known issues with pull request:

None

Change log entries:

None

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@feerrenrut
Prevent ComError escaping when checking for aria-details
8060a6a 
Use _getIA2RelationFirstTarget which falls back to iterating
over _IA2Relations if IA2_2 is not available.
@feerrenrut
Ensure IUnkown is converted to an IAccessible
c83d02d 
Added typing where it could be inferred.
@feerrenrut
Unecessary firefox error log on missing detailsSummary
4359854
@feerrenrut feerrenrut requested a review from a team as a code owner March 10, 2022 13:27
@feerrenrut feerrenrut requested review from seanbudd and removed request for a team March 10, 2022 13:27
seanbudd
seanbudd previously approved these changes Mar 10, 2022
View reviewed changes

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, I've added some missing type suggestions that can be added if they are accurate.

Comment thread source/IAccessibleHandler/__init__.py Outdated
Comment thread source/NVDAObjects/IAccessible/__init__.py
@feerrenrut @seanbudd
Add type for childID
5aabb61 
Co-authored-by: Sean Budd <sean@nvaccess.org>
@feerrenrut

feerrenrut commented Mar 11, 2022

Copy link
Copy Markdown
Contributor Author

Stacked PR's don't get built automatically, I'd like to get confirmation from reporters that this resolves the issue. The following try-build has been started:
https://ci.appveyor.com/project/NVAccess/nvda/builds/42863301

Base automatically changed from branchFor2022.1 to beta March 11, 2022 08:33
@feerrenrut

feerrenrut commented Mar 11, 2022

Copy link
Copy Markdown
Contributor Author

Link to build to test: https://ci.appveyor.com/api/buildjobs/qxi2p6nuywxc6gav/artifacts/output%2Fnvda_snapshot_try-fixFirefoxDetails-24931%2C43598547.exe

This was referenced Mar 11, 2022
Closed
Closed
michaelDCurran
michaelDCurran previously approved these changes Mar 13, 2022
View reviewed changes
@feerrenrut

feerrenrut commented Mar 14, 2022

Copy link
Copy Markdown
Contributor Author

The reporters have indicated on-going errors still. I'll investigate further.

@feerrenrut feerrenrut marked this pull request as draft March 14, 2022 02:06
@feerrenrut

feerrenrut commented Mar 14, 2022

Copy link
Copy Markdown
Contributor Author

New try build to test

@feerrenrut feerrenrut marked this pull request as ready for review March 15, 2022 04:59
@feerrenrut

feerrenrut commented Mar 15, 2022

Copy link
Copy Markdown
Contributor Author

Both reporters have confirmed the issue is resolved.

michaelDCurran
michaelDCurran requested changes Mar 15, 2022
View reviewed changes
Comment thread source/NVDAObjects/IAccessible/__init__.py Outdated
@feerrenrut feerrenrut merged commit 20e030c into beta Mar 15, 2022
@feerrenrut feerrenrut deleted the fixFirefoxDetails branch March 15, 2022 06:45
@nvaccessAuto nvaccessAuto added this to the 2022.2 milestone Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelDCurran michaelDCurran michaelDCurran approved these changes

@seanbudd seanbudd seanbudd left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2022.2

Development

Successfully merging this pull request may close these issues.

4 participants

@feerrenrut @seanbudd @michaelDCurran @nvaccessAuto