539699 - ITypeInfo nsAccessibleWrap member should be static

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

spun off bug 166994 comment #53

Comment 1

[alexander :surkov (:asurkov)]

turn nsAccessibleWrap::mTypeInfo (http://mxr.mozilla.org/mozilla-central/source/accessible/src/msaa/nsAccessibleWrap.h#361) into static member

Comment 2

[Andrew Quartey [:drexler]]

Attachment: Patch v1

https://tbpl.mozilla.org/?tree=Try&rev=ff3e7cef0591

Comment 3

[Hubert Figuiere [:hub]]

Comment on attachment 582569 [details] [diff] [review]
Patch v1

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

::: accessible/src/msaa/nsAccessibleWrap.h
@@ -358,4 +358,4 @@
> >     */
> >    ITypeInfo *GetTI(LCID lcid);
> >  
> > -  ITypeInfo *mTypeInfo;
> > +  static ITypeInfo *mTypeInfo;

> > +  static ITypeInfo *mTypeInfo;

The coding style in accessibility would be to write ITypeInfo* (the * stuck to the type, space after)

Comment 4

[Andrew Quartey [:drexler]]

Attachment: Patch v2

Coding style changes per Hub's suggestion.

Comment 5

[Trevor Saunders (:tbsaunde)]

Comment on attachment 582588 [details] [diff] [review]
Patch v2


>-  ITypeInfo *GetTI(LCID lcid);
>+  ITypeInfo* GetTI(LCID lcid);

make it static too.

Comment 6

[Andrew Quartey [:drexler]]

Attachment: Patch v3

made GetTI() a static function although i think it's not that necessary since the only current callers are all members of nsAccessibleWrap.

Comment 7

[Hubert Figuiere [:hub]]

(In reply to andrew (:drexler) from comment #6)

> made GetTI() a static function although i think it's not that necessary
> since the only current callers are all members of nsAccessibleWrap.

static for a C++ method has nothing to do with visibility (ie who calls it).

Comment 8

[Trevor Saunders (:tbsaunde)]

Comment on attachment 583707 [details] [diff] [review]
Patch v3

review isn't necessary here since already granted.

Comment 9

[Trevor Saunders (:tbsaunde)]

Comment on attachment 583707 [details] [diff] [review]
Patch v3

Hub, its fine in this case since review was already granted, and the patch is fine, but in general only the module owner / peers or someone they request review from should grant it.

Comment 10

[Trevor Saunders (:tbsaunde)]

I was about to check this in, but realized that I somehow missed asking you to change the name from mTypeInfo to gTypeInfo since its now static

Comment 11

[Andrew Quartey [:drexler]]

Attachment: Patch v4

mTypeInfo renamed to gTypeInfo

Comment 12

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/projects/accessibility/rev/648239b7558d
thanks for the patch!

Comment 13

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/mozilla-central/rev/648239b7558d

thanks!