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

fix build Warnings in editor/

RESOLVED FIXED in mozilla12

Status

()

Core
Editor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Atul Aggarwal, Assigned: Atul Aggarwal)

Tracking

Trunk
mozilla12
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 575140 [details]
Warnings

Patch coming shortly.
(Assignee)

Comment 1

6 years ago
Created attachment 575141 [details] [diff] [review]
Patch v1
Assignee: nobody → atulagrwl
Attachment #575141 - Flags: review?(ehsan)
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.
Attachment #575141 - Flags: review?(ehsan) → review+
(Assignee)

Comment 3

6 years ago
Created attachment 577594 [details] [diff] [review]
Patch v1.10
Attachment #575141 - Attachment is obsolete: true
(Assignee)

Comment 4

6 years ago
(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 !
(Assignee)

Updated

6 years ago
Attachment #577594 - Flags: review?(ehsan)
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!
Attachment #577594 - Flags: review?(ehsan) → review+
(Assignee)

Comment 6

6 years ago
Thanks for reviewing :).
Keywords: checkin-needed
patching file editor/libeditor/html/nsHTMLEditor.cpp
Hunk #1 FAILED at 932

:(
Keywords: checkin-needed
(Assignee)

Comment 8

6 years ago
Created attachment 583847 [details] [diff] [review]
Patch v1.50

rebase on current code.
Attachment #577594 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f0062478a1f
Flags: in-testsuite-
Keywords: checkin-needed
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla12
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/1f0062478a1f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.