Closed Bug 583076 Opened 10 years ago Closed 10 years ago

Fix a number of build warnings in accessible/

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

(Whiteboard: [build_warning])

Attachments

(1 file)

Attached patch Patch v1Splinter Review
In particular, a number of warnings due to functions in base classes being hidden, unmatched enum values in ?: expressions, and unused variables. Patch attached.
Attachment #461335 - Flags: review?(surkov.alexander)
Comment on attachment 461335 [details] [diff] [review]
Patch v1


> nsAccTextChangeEvent::
>   nsAccTextChangeEvent(nsAccessible *aAccessible, PRInt32 aStart,
>                        nsAString& aModifiedText, PRBool aIsInserted,
>-                       PRBool aIsAsynch, EIsFromUserInput aIsFromUserInput) :
>-  nsAccEvent(aIsInserted ? nsIAccessibleEvent::EVENT_TEXT_INSERTED : nsIAccessibleEvent::EVENT_TEXT_REMOVED,
>-             aAccessible, aIsAsynch, aIsFromUserInput, eAllowDupes),
>-  mStart(aStart), mIsInserted(aIsInserted), mModifiedText(aModifiedText)
>+                       PRBool aIsAsynch, EIsFromUserInput aIsFromUserInput)
>+  : nsAccEvent(aIsInserted
>+               ? static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_INSERTED)
>+               : static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_REMOVED),

nit: do not keep operators at the begin of line, it should be 

> 
>+#include "mozilla/unused.h"
>+

> PRInt32
> nsAccessible::GetIndexInParent()
> {
>   // XXX: call GetParent() to repair the tree if it's broken.
>-  nsAccessible* parent = GetParent();
>+  mozilla::unused << GetParent();
>   return mIndexInParent;

GetParent() is enough here.
Attachment #461335 - Flags: review?(surkov.alexander) → review+
(In reply to comment #1)

> nit: do not keep operators at the begin of line, it should be 

nsAccEvent(aIsInserted ?
  static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_INSERTED) :
  static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_REMOVED),
Comment on attachment 461335 [details] [diff] [review]
Patch v1

(In reply to comment #1)
> Comment on attachment 461335 [details] [diff] [review]
> > 
> >+#include "mozilla/unused.h"
> >+
> 
> > PRInt32
> > nsAccessible::GetIndexInParent()
> > {
> >   // XXX: call GetParent() to repair the tree if it's broken.
> >-  nsAccessible* parent = GetParent();
> >+  mozilla::unused << GetParent();
> >   return mIndexInParent;
> 
> GetParent() is enough here.

Ok. (Eventually I'd like to get warnings for ignoring what getters return, but that can wait.)

(In reply to comment #2)
> (In reply to comment #1)
> 
> > nit: do not keep operators at the begin of line, it should be 
> 
> nsAccEvent(aIsInserted ?
>   static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_INSERTED) :
>   static_cast<PRUint32>(nsIAccessibleEvent::EVENT_TEXT_REMOVED),

Ok.
Attachment #461335 - Flags: approval2.0?
(In reply to comment #3)

> > >   // XXX: call GetParent() to repair the tree if it's broken.
> > >-  nsAccessible* parent = GetParent();
> > >+  mozilla::unused << GetParent();
> > >   return mIndexInParent;
> > 
> > GetParent() is enough here.
> 
> Ok. (Eventually I'd like to get warnings for ignoring what getters return, but
> that can wait.)

then it's ok
Attachment #461335 - Flags: approval2.0? → approval2.0+
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/3900417b9594
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.