Note: There are a few cases of duplicates in user autocompletion which are being worked on.

ITypeInfo nsAccessibleWrap member should be static

RESOLVED FIXED in mozilla12

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: surkov, Assigned: drexler)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla12
All
Windows XP
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
spun off bug 166994 comment #53
(Reporter)

Comment 1

6 years ago
turn nsAccessibleWrap::mTypeInfo (http://mxr.mozilla.org/mozilla-central/source/accessible/src/msaa/nsAccessibleWrap.h#361) into static member
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
(Assignee)

Comment 2

6 years ago
Created attachment 582569 [details] [diff] [review]
Patch v1

https://tbpl.mozilla.org/?tree=Try&rev=ff3e7cef0591
Assignee: nobody → andrew.quartey
Attachment #582569 - Flags: review?(surkov.alexander)
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)
(Assignee)

Comment 4

6 years ago
Created attachment 582588 [details] [diff] [review]
Patch v2

Coding style changes per Hub's suggestion.
Attachment #582569 - Attachment is obsolete: true
Attachment #582588 - Flags: review?(surkov.alexander)
Attachment #582569 - Flags: review?(surkov.alexander)
Comment on attachment 582588 [details] [diff] [review]
Patch v2


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

make it static too.
Attachment #582588 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 6

6 years ago
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.
Attachment #582588 - Attachment is obsolete: true
Attachment #583707 - Flags: review?(trev.saunders)
(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).

Updated

6 years ago
Attachment #583707 - Flags: review+
Comment on attachment 583707 [details] [diff] [review]
Patch v3

review isn't necessary here since already granted.
Attachment #583707 - Flags: review?(trev.saunders) → review+
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.
Attachment #583707 - Flags: review+
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
(Assignee)

Comment 11

6 years ago
Created attachment 586166 [details] [diff] [review]
Patch v4

mTypeInfo renamed to gTypeInfo
Attachment #583707 - Attachment is obsolete: true
https://hg.mozilla.org/projects/accessibility/rev/648239b7558d
thanks for the patch!
https://hg.mozilla.org/mozilla-central/rev/648239b7558d

thanks!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.