Closed Bug 382296 Opened 13 years ago Closed 13 years ago

NIT: reduce compile warnings with gcc in a11y module

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

Details

Attachments

(1 file)

There're several compile warnings when compiling mozilla/accessible module.

That's a little annoying and preventing us to find real warnings.
Attached patch patchSplinter Review
also fixed "*aExtraState =" to "*aExtraState |=" in nsAccessible.cpp
Attachment #266449 - Flags: review?(surkov.alexander)
Comment on attachment 266449 [details] [diff] [review]
patch

>-    PRUint32 geckoCoordType = (aCoords == ATK_XY_SCREEN) ?
>-        nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE :
>-        nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE;
>+    PRUint32 geckoCoordType;
>+    if (aCoords == ATK_XY_SCREEN)
>+        geckoCoordType = nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE;
>+    else
>+        geckoCoordType = nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE;

What's kind of error does '? : ' statement bring?

> nsCaretAccessible::nsCaretAccessible(nsIDOMNode* aDocumentNode, nsIWeakReference* aShell, nsRootAccessible *aRootAccessible):
>-nsLeafAccessible(aDocumentNode, aShell), mLastCaretOffset(-1), mLastNodeWithCaret(nsnull),
>-mCurrentControl(nsnull), mRootAccessible(aRootAccessible)
>+nsLeafAccessible(aDocumentNode, aShell), mCurrentControl(nsnull), mLastNodeWithCaret(nsnull),
>+mLastCaretOffset(-1), mRootAccessible(aRootAccessible)

Why is order important?

>-  nsresult rv = nsLinkableAccessible::GetState(aState, aExtraState);
>+  nsLinkableAccessible::GetState(aState, aExtraState);

IIRC usually we do not continue if base GetState() failed.
(In reply to comment #2)
> What's kind of error does '? : ' statement bring?

warning: enumeral mismatch in conditional expression: ‘nsIAccessibleCoordinateType::<anonymous enum>’ vs ‘nsIAccessibleCoordinateType::<anonymous enum>’
 
> > nsCaretAccessible::nsCaretAccessible(nsIDOMNode* aDocumentNode, nsIWeakReference* aShell, nsRootAccessible *aRootAccessible):
> >-nsLeafAccessible(aDocumentNode, aShell), mLastCaretOffset(-1), mLastNodeWithCaret(nsnull),
> >-mCurrentControl(nsnull), mRootAccessible(aRootAccessible)
> >+nsLeafAccessible(aDocumentNode, aShell), mCurrentControl(nsnull), mLastNodeWithCaret(nsnull),
> >+mLastCaretOffset(-1), mRootAccessible(aRootAccessible)
> 
> Why is order important?

nsCaretAccessible.h: In constructor ‘nsCaretAccessible::nsCaretAccessible(nsIDOMNode*, nsIWeakReference*, nsRootAccessible*)’:
nsCaretAccessible.h:115: warning: ‘nsCaretAccessible::mLastCaretOffset’ will be initialized after
nsCaretAccessible.h:114: warning:   ‘nsCOMPtr<nsIDOMNode> nsCaretAccessible::mLastNodeWithCaret’
nsCaretAccessible.cpp:58: warning:   when initialized here
nsCaretAccessible.h:114: warning: ‘nsCaretAccessible::mLastNodeWithCaret’ will be initialized after
nsCaretAccessible.h:107: warning:   ‘nsCOMPtr<nsIDOMNode> nsCaretAccessible::mCurrentControl’
nsCaretAccessible.cpp:58: warning:   when initialized here
 
> >-  nsresult rv = nsLinkableAccessible::GetState(aState, aExtraState);
> >+  nsLinkableAccessible::GetState(aState, aExtraState);
> 
> IIRC usually we do not continue if base GetState() failed.
>
ok, so the problem here is we missed NS_ENSURE_SUCCESS(rv, rv);
Right?


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

(In reply to comment #3)

> ok, so the problem here is we missed NS_ENSURE_SUCCESS(rv, rv);
> Right?
> 

right.
Attachment #266449 - Flags: review?(surkov.alexander) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.