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

Incorrect use of PRBool when other types are more appropriate (or vice versa)

RESOLVED FIXED in mozilla8

Status

()

Core
Rewriting and Analysis
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

(Depends on: 1 bug)

Trunk
mozilla8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Fix
10.91 KB, patch
Ehsan
: review+
dwitte@gmail.com
: review+
Joe Drew (not getting mail)
: review+
vlad
: review+
luke
: review+
mak
: review+
roc
: review+
Biesinger
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 545750 [details] [diff] [review]
Fix

Problems discovered by static analysis and patched by hand.

Reviewers this round:

bz:
 content/base/src/nsGenericElement.cpp          |    2 +-
 content/base/src/nsLineBreaker.cpp             |    4 ++--
 docshell/base/nsDocShell.cpp                   |    4 ++--
 layout/xul/base/src/nsListBoxBodyFrame.cpp     |    2 +-
 layout/xul/base/src/nsMenuPopupFrame.h         |    2 +-

ehsan:
 editor/libeditor/html/nsHTMLEditRules.cpp      |    2 +-
 editor/libeditor/html/nsHTMLEditRules.h        |    2 +-
 editor/libeditor/html/nsTableEditor.cpp        |    4 ++--

dwitte:
 extensions/cookie/nsCookiePermission.cpp       |    5 +++--

bholley:
 gfx/thebes/gfxPlatform.cpp                     |    2 +-

vlad:
 intl/uconv/src/nsUTF8ToUnicode.cpp             |    2 +-

luke:
 js/src/jstracer.cpp                            |    2 +-
 js/src/methodjit/StubCalls.cpp                 |    2 +-

biesi:
 netwerk/base/src/nsLoadGroup.cpp               |    4 ++--

mak:
 toolkit/components/places/nsNavHistoryResult.h |    2 +-

roc:
 widget/src/xpwidgets/nsPrintSettingsImpl.cpp   |    2 +-
 widget/src/xpwidgets/nsPrintSettingsImpl.h     |    2 +-
Attachment #545750 - Flags: review?(vladimir)
Attachment #545750 - Flags: review?(roc.
Attachment #545750 - Flags: review?(mak77)
Attachment #545750 - Flags: review?(luke)
Attachment #545750 - Flags: review?(ehsan)
Attachment #545750 - Flags: review?(dwitte)
Attachment #545750 - Flags: review?(cbiesinger)
Attachment #545750 - Flags: review?(bzbarsky)
Attachment #545750 - Flags: review?(bobbyholley+bmo)

Updated

6 years ago
Attachment #545750 - Flags: review?(luke) → review+
Comment on attachment 545750 [details] [diff] [review]
Fix

Review of attachment 545750 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #545750 - Flags: review?(roc) → review+

Comment 2

6 years ago
Comment on attachment 545750 [details] [diff] [review]
Fix

>+++ b/content/base/src/nsLineBreaker.cpp

You need to change nsLineBreaker.h too.

>+++ b/docshell/base/nsDocShell.cpp
>+static PRUint32 gValidateOrigin = 0xffffffff;

Excellent.  We couldn't do this before for some reason....

>+++ b/layout/xul/base/src/nsMenuPopupFrame.h
>+  PRInt8 mConsumeRollupEvent; // Should the rollup event be consumed?

You need to change the constructor so that initialization order matches declaration order.

Also, fix the comment?  Perhaps to say that it's one of nsIPopupBoxObject::ROLLUP_DEFAULT/ROLLUP_CONSUME/ROLLUP_NO_CONSUME ?

r=me on my part with the above nits.
Attachment #545750 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

6 years ago
Comment on attachment 545750 [details] [diff] [review]
Fix

Clearing review for the netwerk part because (nssupportsarray)AppendElement doesn't actually return nsresult, as bz points out. Will open another bug to fix that insanity.
Attachment #545750 - Flags: review?(cbiesinger)
Attachment #545750 - Flags: review?(bobbyholley+bmo) → review?(joe)
Comment on attachment 545750 [details] [diff] [review]
Fix

r+ for nsUTF8ToUnicode (can I even r+ that? I know the original/changed code was mine...)
Attachment #545750 - Flags: review?(vladimir) → review+
Comment on attachment 545750 [details] [diff] [review]
Fix

r=me for the editor parts.
Attachment #545750 - Flags: review?(ehsan) → review+
Comment on attachment 545750 [details] [diff] [review]
Fix

Review of attachment 545750 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #545750 - Flags: review?(mak77) → review+
Comment on attachment 545750 [details] [diff] [review]
Fix

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

::: gfx/thebes/gfxPlatform.cpp
@@ +927,4 @@
>  #define INTENT_MIN 0
>  #define INTENT_MAX 3
>  
> +int

This needs to be changed in gfxPlatform.h too.
Attachment #545750 - Flags: review?(joe) → review+
Attachment #545750 - Flags: review+
(Assignee)

Comment 8

6 years ago
Comment on attachment 545750 [details] [diff] [review]
Fix

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

::: gfx/thebes/gfxPlatform.cpp
@@ +927,4 @@
>  #define INTENT_MIN 0
>  #define INTENT_MAX 3
>  
> +int

gfxPlatform.h already specifies this as int, AFAICT.

Updated

6 years ago
Attachment #545750 - Flags: review?(dwitte) → review+
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/4d170faf6122
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/4d170faf6122
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
(Assignee)

Updated

6 years ago
Duplicate of this bug: 398581
You need to log in before you can comment on or make changes to this bug.