703239 - fix build Warnings in editor/

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Atul Aggarwal]

Attachment: Warnings

Patch coming shortly.

Comment 1

[Atul Aggarwal]

Attachment: Patch v1

Comment 2

[(no longer active)]

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

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

This is great!  r=me with the comments below.

::: editor/composer/src/nsComposerController.cpp
@@ +44,5 @@
>  
>  #define NS_REGISTER_ONE_COMMAND(_cmdClass, _cmdName)                    \
>    {                                                                     \
>      _cmdClass* theCmd = new _cmdClass();                                \
> +    NS_ENSURE_TRUE(theCmd, NS_ERROR_OUT_OF_MEMORY);                     \

Now that you're here, can you please remove this NS_ENSURE_TRUE (and others below like it) too?  operator new is infallible these days.

Thanks!

::: editor/libeditor/html/nsHTMLEditor.cpp
@@ +935,5 @@
>  {
>    if (!aNode)
>    {
>      NS_NOTREACHED("null node passed to GetBlockNodeParent()");
> +    return nsnull;

Ouch!

::: editor/libeditor/html/nsHTMLEditorStyle.cpp
@@ +672,1 @@
>          (!aProperty && NodeIsProperty(aNode)))  // or node is any prop and we asked for that

Please remove the double parenthesis after if.  Also, remove the trailing space after '(' on the first line.

Comment 3

[Atul Aggarwal]

Attachment: Patch v1.10

Comment 4

[Atul Aggarwal]

(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> Comment on attachment 575141 [details] [diff] [review] [diff] [details] [review]
> Patch v1
> 
> Review of attachment 575141 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> This is great!  r=me with the comments below.
> 
> ::: editor/composer/src/nsComposerController.cpp
> Now that you're here, can you please remove this NS_ENSURE_TRUE (and others
> below like it) too?  operator new is infallible these days.
> 
Done.
> 
> ::: editor/libeditor/html/nsHTMLEditor.cpp
> @@ +935,5 @@
> >  {
> >    if (!aNode)
> >    {
> >      NS_NOTREACHED("null node passed to GetBlockNodeParent()");
> > +    return nsnull;
> 
> Ouch!
This was intentional. Did I miss something?
> 
> ::: editor/libeditor/html/nsHTMLEditorStyle.cpp
> @@ +672,1 @@
> >          (!aProperty && NodeIsProperty(aNode)))  // or node is any prop and we asked for that
> 
> Please remove the double parenthesis after if.  Also, remove the trailing
> space after '(' on the first line.

Removed the trailing space but double parenthesis was intentional to shut compiler from throwing warnings.

Please review so that I can mark this bug checkin-needed.

Thanks !

Comment 5

[(no longer active)]

Comment on attachment 577594 [details] [diff] [review]
Patch v1.10

The "ouch" was because we'd missed that case all this time!  Great work, thanks a lot!

Comment 6

[Atul Aggarwal]

Thanks for reviewing :).

Comment 7

[Dão Gottwald [:dao]]

patching file editor/libeditor/html/nsHTMLEditor.cpp
Hunk #1 FAILED at 932

:(

Comment 8

[Atul Aggarwal]

Attachment: Patch v1.50

rebase on current code.

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f0062478a1f

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1f0062478a1f