648267 - remove nsIWinAccessNode interface

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

When bug 641838 and 648265 are fixed then we don't need this interface and can call IUknown::QueryInterface directly on nsAccessible instance.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 702532 [details] [diff] [review]
patch

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

r=me with comments fixed

::: accessible/src/msaa/nsAccessNodeWrap.h
@@ +57,4 @@
>                           public IServiceProvider
>  {
>    public:
>      NS_DECL_ISUPPORTS_INHERITED

it doesn't seem you need ISupports definitons on nsAccessNodeWrap if I don't miss anything

::: accessible/src/windows/ia2/ia2AccessibleHypertext.cpp
@@ +49,3 @@
>      return E_FAIL;
>  
> +  (*aHyperlink = static_cast<AccessibleWrap*>(hyperLink))->AddRef();

I prefer to preserve casting to IAccessibleHyperlink. Sometimes the difference matters

also I would avoid constructions like (a = b)->Method() they seem complicated

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #2)
> Comment on attachment 702532 [details] [diff] [review]
> patch
> 
> Review of attachment 702532 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with comments fixed
> 
> ::: accessible/src/msaa/nsAccessNodeWrap.h
> @@ +57,4 @@
> >                           public IServiceProvider
> >  {
> >    public:
> >      NS_DECL_ISUPPORTS_INHERITED
> 
> it doesn't seem you need ISupports definitons on nsAccessNodeWrap if I don't
> miss anything

I'm not really sure, it still inherits from nsISupports (through nsAccessNode) and IUnknown (from IServiceProvider).  Your probably right, but I already have a patch on top of this one moving IServiceProvider out of nsAccessNodeWrap, and it seems easier to nuke this stuff after that than risk bit rotting myself.

Comment 4

[alexander :surkov (:asurkov)]

ok, fine with me

Comment 5

[Trevor Saunders (:tbsaunde)]

Attachment: follow up really nuke the dead accessibility-msaa xpt

all deletions :-)

Comment 6

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d7b1e416505

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4d7b1e416505

Comment 8

[Florian Quèze [:florian]]

Attachment: Stop packaging accessibility-msaa.xpt in comm-central

Mark, I think this should fix the current bustage on Windows.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 714283 [details] [diff] [review]
Stop packaging accessibility-msaa.xpt in comm-central

I should read my email before landing bustage fixes ;-)

https://hg.mozilla.org/comm-central/rev/8a074bb38774

landed the mail part of this, but r+ for the other parts.

In general if they are just as simple in future, you have a blanket rs=me for landing bustage fixes like this.

Comment 10

[Frank Wein [:mcsmurf]]

I also landed the suite/ change now myself :o well, calendar is still left :-)

Comment 11

[Philip Chee]

> I also landed the suite/ change now myself :o well, calendar is still left :-)
For the record this is the changeset Frank landed:
http://hg.mozilla.org/comm-central/rev/28fcd1fd1fb4