Closed Bug 648267 Opened 9 years ago Closed 7 years ago

remove nsIWinAccessNode interface

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: surkov, Assigned: tbsaunde)

References

Details

(Keywords: access)

Attachments

(3 files)

When bug 641838 and 648265 are fixed then we don't need this interface and can call IUknown::QueryInterface directly on nsAccessible instance.
Depends on: 741701
Depends on: 743654
Depends on: 767272, 829382
Attached patch patchSplinter Review
Attachment #702532 - Flags: review?(surkov.alexander)
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
Attachment #702532 - Flags: review?(surkov.alexander) → review+
(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.
ok, fine with me
all deletions :-)
Attachment #712637 - Flags: review?(mh+mozilla)
Attachment #712637 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/4d7b1e416505
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Mark, I think this should fix the current bustage on Windows.
Attachment #714283 - Flags: review?(mbanner)
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.
Attachment #714283 - Flags: review?(mbanner) → review+
I also landed the suite/ change now myself :o well, calendar is still left :-)
> 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
Depends on: 841928
You need to log in before you can comment on or make changes to this bug.