Last Comment Bug 539699 - ITypeInfo nsAccessibleWrap member should be static
: ITypeInfo nsAccessibleWrap member should be static
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All Windows XP
: -- normal (vote)
: mozilla12
Assigned To: Andrew Quartey [:drexler]
:
Mentors:
Depends on: 166994
Blocks: cleana11y
  Show dependency treegraph
 
Reported: 2010-01-14 07:47 PST by alexander :surkov
Modified: 2012-01-11 00:07 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (2.01 KB, patch)
2011-12-17 11:54 PST, Andrew Quartey [:drexler]
no flags Details | Diff | Splinter Review
Patch v2 (2.19 KB, patch)
2011-12-17 13:32 PST, Andrew Quartey [:drexler]
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review
Patch v3 (2.20 KB, patch)
2011-12-21 19:19 PST, Andrew Quartey [:drexler]
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review
Patch v4 (3.22 KB, patch)
2012-01-05 11:49 PST, Andrew Quartey [:drexler]
no flags Details | Diff | Splinter Review

Description alexander :surkov 2010-01-14 07:47:53 PST
spun off bug 166994 comment #53
Comment 1 alexander :surkov 2011-12-07 09:15:55 PST
turn nsAccessibleWrap::mTypeInfo (http://mxr.mozilla.org/mozilla-central/source/accessible/src/msaa/nsAccessibleWrap.h#361) into static member
Comment 2 Andrew Quartey [:drexler] 2011-12-17 11:54:15 PST
Created attachment 582569 [details] [diff] [review]
Patch v1

https://tbpl.mozilla.org/?tree=Try&rev=ff3e7cef0591
Comment 3 Hubert Figuiere [:hub] 2011-12-17 12:08:27 PST
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] 2011-12-17 13:32:57 PST
Created attachment 582588 [details] [diff] [review]
Patch v2

Coding style changes per Hub's suggestion.
Comment 5 Trevor Saunders (:tbsaunde) 2011-12-21 11:19:18 PST
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] 2011-12-21 19:19:11 PST
Created attachment 583707 [details] [diff] [review]
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] 2011-12-22 10:15:40 PST
(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) 2011-12-22 12:02:44 PST
Comment on attachment 583707 [details] [diff] [review]
Patch v3

review isn't necessary here since already granted.
Comment 9 Trevor Saunders (:tbsaunde) 2011-12-22 12:22:04 PST
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) 2011-12-29 15:45:39 PST
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] 2012-01-05 11:49:33 PST
Created attachment 586166 [details] [diff] [review]
Patch v4

mTypeInfo renamed to gTypeInfo
Comment 12 Trevor Saunders (:tbsaunde) 2012-01-06 14:38:23 PST
https://hg.mozilla.org/projects/accessibility/rev/648239b7558d
thanks for the patch!
Comment 13 Trevor Saunders (:tbsaunde) 2012-01-10 18:30:44 PST
https://hg.mozilla.org/mozilla-central/rev/648239b7558d

thanks!

Note You need to log in before you can comment on or make changes to this bug.