808229 - rm a11yGeneric.h

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

its all garbage

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: part 1 unused runnable macros

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: part 2 inline the remaining runnable macros

They're only used once, so its simpler and more contained to just define the class we actually need where we need it

Comment 3

[Trevor Saunders (:tbsaunde)]

Attachment: part 3 unused NS_ENSURE_ macr

success error codes are madness and this one isn't even used!

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: part 4 inline interface map macro

I'm fairly confident there's an existing macro that would be fine here, but I don't really want to dig to much looking, and just inlining it works just fine

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: part 5 rm a11ygeneric.h

its totally unused now

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 677935 [details] [diff] [review]
part 2 inline the remaining runnable macros

Review of attachment 677935 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/generic/Accessible.cpp
@@ +2100,5 @@
>  
>  void
>  Accessible::DoCommand(nsIContent *aContent, uint32_t aActionIndex)
>  {
>    nsIContent* content = aContent ? aContent : mContent.get();

please move it where it's used, i.e. right before nsCOMPtr<nsIRunnable>

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 677939 [details] [diff] [review]
part 5 rm a11ygeneric.h

Review of attachment 677939 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/a11yGeneric.h
@@ -12,5 @@
> -// but some of our classes have an ambiguous base class of nsISupports which
> -// prevents this from working (the default macro converts it to nsISupports,
> -// then addrefs it, then returns it). Therefore, we expand the macro here and
> -// change it so that it works. Yuck.
> -#define NS_INTERFACE_MAP_STATIC_AMBIGUOUS(_class)                              \

I prefer to keep it and move it somewhere (I know we get less xpcomish and we have a single use case in a11y but anyway I would like to keep this trick explicit). Perhaps the best place is in nsISupportsImpl.h since it's used not in a11y only

Comment 8

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #7)
> Comment on attachment 677939 [details] [diff] [review]
> part 5 rm a11ygeneric.h
> 
> Review of attachment 677939 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/a11yGeneric.h
> @@ -12,5 @@
> > -// but some of our classes have an ambiguous base class of nsISupports which
> > -// prevents this from working (the default macro converts it to nsISupports,
> > -// then addrefs it, then returns it). Therefore, we expand the macro here and
> > -// change it so that it works. Yuck.
> > -#define NS_INTERFACE_MAP_STATIC_AMBIGUOUS(_class)                              \
> 
> I prefer to keep it and move it somewhere (I know we get less xpcomish and
> we have a single use case in a11y but anyway I would like to keep this trick
> explicit). Perhaps the best place is in nsISupportsImpl.h since it's used
> not in a11y only

ok, so I looked a little deeper, and it turns out this is pretty funny we claim there to be implementing the IID XULTreeGridRowAccessible, but never declare any such thing.  but XULTreeGridRowAccessible inherits from XULTreeItemAccessibleBase which does have a IID.  NS_GET_IID() is a macro defined like this NS_GET_IID(T) ::T::ComTypeInfo<int>::kIID so, I'm going to guess that the actual iid we end up with there is the one for XULTreeItemAccessibleBase which we would have claimed to support anyway by up calling its QI.  So I expect I can just remove that code all together.

Comment 9

[alexander :surkov (:asurkov)]

Yes, that's fine

Comment 10

[Trevor Saunders (:tbsaunde)]

Attachment: part 4 v2

just remove the nonsense

Comment 11

[Trevor Saunders (:tbsaunde)]

and, yes it infact passes tests :)

Comment 12

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #11)
> and, yes it infact passes tests :)

it's a good sign since XUL trees are more or less covered

Comment 13

[Trevor Saunders (:tbsaunde)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/508f6cb9eaa8
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/999673b552f4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/77fa19b95a71
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/90f23642624f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a81e93520c49

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/508f6cb9eaa8
https://hg.mozilla.org/mozilla-central/rev/999673b552f4
https://hg.mozilla.org/mozilla-central/rev/77fa19b95a71
https://hg.mozilla.org/mozilla-central/rev/90f23642624f
https://hg.mozilla.org/mozilla-central/rev/a81e93520c49