1409541 - ArrayData fixes

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

We should make some improvements to sPlatformChildArrayData that will help with both perf and correctness. In particular, we should get rid of any instances of mArrayParamIid == IID_IUnknown and use the expected IID instead.

Also I think we're missing an entry for IAccessibleTable2::get_selectedCells.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Fixes to array data

This patch adds a missing ArrayData entry for IAccessibleTable2::get_selectedCells.

It also replaces all occurrences of IID_IUnknown with the real, expected interface IIDs (The field in question represents the IID of the interface in the output array).

See ipc/mscom/Registration.h for the declaration of the ArrayData structure.

Comment 2

[James Teh [:Jamie]]

Comment on attachment 8919481 [details] [diff] [review]
Fixes to array data

Looks good!

I think it'd be useful to have comments on the indexes indicating the method/argument names, since counting them is tedious and error prone (unless you know of some more automated way that I don't?). However, this code already existed, so that's not so much a comment on this patch and more of a reminder to consider this for future stuff like this.

Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ee54768edd3
Replace IID_IUnknown with expected interfaces for outparams in a11y content ArrayData. r=Jamie

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1ee54768edd3

Comment 6

[Alexandru Simonca [:asimonca], Release Desktop QA]

I haven't crashed all day using NVDA on big pages. Seems fixed. Marking accordingly.

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Aaron, would you consider uplifting the fix for bug 1409541 to beta57? I don't see any crash reports on 57 in the last 7 days. Given that, should we just wontfix Bug 1410407 for 57?

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

This is a dependency of bug 1406822 which has an uplift request. If we decide to uplift that, then this bug will need to ride along too.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8919481 [details] [diff] [review]
Fixes to array data

Got it. Thanks Aaron. This fix is needed to fix 57 blocking bug 1406822, beta57+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/349332cc8d85

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://hg.mozilla.org/releases/mozilla-beta/rev/8b5e439d94d87dc3ef933e21d3ebb359538fbfc1

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/745e43737c95

Comment 13

[Alexandru Simonca [:asimonca], Release Desktop QA]

Verified fixed using 57.b13 on Windows 10 x64 and Windows 7 x64.