The default bug view has changed. See this FAQ.

Implement IAccessible2_2::relationTargetsOfType

RESOLVED FIXED in mozilla27

Status

()

Core
Disability Access APIs
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla27
All
Windows 7
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

5.93 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
http://accessibility.linuxfoundation.org/a11yspecs/ia2/docs/html/interface_i_accessible2__2.html

should be a plain mapping into Accessible::RelationByType
(Assignee)

Updated

4 years ago
No longer blocks: 614570
Depends on: 614570
(Assignee)

Updated

4 years ago
Blocks: 368873
(Assignee)

Comment 1

4 years ago
Created attachment 813545 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Attachment #813545 - Flags: review?(trev.saunders)
(Assignee)

Comment 2

4 years ago
Created attachment 818796 [details] [diff] [review]
patch2
Attachment #813545 - Attachment is obsolete: true
Attachment #813545 - Flags: review?(trev.saunders)
Attachment #818796 - Flags: review?(trev.saunders)
Comment on attachment 818796 [details] [diff] [review]
patch2

>+  if (!Compatibility::IsIA2Off()) {

IA2Off is just a hack for really really ancient jaws so why don't we make it only effect IAccessible2 itself?

>+    if (IID_IAccessible2 == iid) {
>+      *ppv = static_cast<IAccessible2*>(this);
>+
>+    } else if (IID_IAccessible2_2 == iid) {

it seems like it would be better to check for the newer interface first

>+  bool matched = false;
>+  RelationType relationType;

why not use the mayBe<RelationType> trick?

>+  for (uint32_t idx = 0; idx < ArrayLength(sRelationTypePairs); idx++) {

size_t ?

>+    if (wcscmp(aType, sRelationTypePairs[idx].second) == 0) {
>+      relationType = sRelationTypePairs[idx].first;
>+      matched = true;

might as well break right?

>+  nsTArray<Accessible*> targets;
>+  Accessible* target = nullptr;
>+  while ((target = rel.Next()) &&
>+         static_cast<long>(targets.Length()) <= aMaxTargets)
>+    targets.AppendElement(target);

what happens if max targets is 0?

>+  for (int32_t i = 0; i < *aNTargets; i++) {

wouldn't long make more sense than int32_t? does it matter on win64 maybe?

>+  // IAccessible2_2
>+  virtual /* [propget] */ HRESULT STDMETHODCALLTYPE get_attribute(
>+    /* [in] */ BSTR name,
>+    /* [out, retval] */ VARIANT* attribute);

any reason to not make them override?
Attachment #818796 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 4

4 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #3)

> >+  if (!Compatibility::IsIA2Off()) {
> 
> IA2Off is just a hack for really really ancient jaws so why don't we make it
> only effect IAccessible2 itself?

ok

> >+    if (IID_IAccessible2 == iid) {
> >+      *ppv = static_cast<IAccessible2*>(this);
> >+
> >+    } else if (IID_IAccessible2_2 == iid) {
> 
> it seems like it would be better to check for the newer interface first

hopefully this interface will picked up :)

> >+  bool matched = false;
> >+  RelationType relationType;
> 
> why not use the mayBe<RelationType> trick?

ok

> >+  for (uint32_t idx = 0; idx < ArrayLength(sRelationTypePairs); idx++) {
> 
> size_t ?

why?

> 
> >+    if (wcscmp(aType, sRelationTypePairs[idx].second) == 0) {
> >+      relationType = sRelationTypePairs[idx].first;
> >+      matched = true;
> 
> might as well break right?
> 
> >+  nsTArray<Accessible*> targets;
> >+  Accessible* target = nullptr;
> >+  while ((target = rel.Next()) &&
> >+         static_cast<long>(targets.Length()) <= aMaxTargets)
> >+    targets.AppendElement(target);
> 
> what happens if max targets is 0?

no targets returned

> >+  for (int32_t i = 0; i < *aNTargets; i++) {
> 
> wouldn't long make more sense than int32_t? does it matter on win64 maybe?

why?

> >+  // IAccessible2_2
> >+  virtual /* [propget] */ HRESULT STDMETHODCALLTYPE get_attribute(
> >+    /* [in] */ BSTR name,
> >+    /* [out, retval] */ VARIANT* attribute);
> 
> any reason to not make them override?

you mean MOZ_OVERRIDE? it's just a copy of what IDL generates, why to care?
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/98614ea8ce7f
https://hg.mozilla.org/mozilla-central/rev/98614ea8ce7f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.