PRBool misuse bugs in editor/

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
11 years ago
7 years ago

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
These issues were found by prcheck.

I'm not sure about
rv = mDocStateListeners[i]->NotifyDocumentStateChanged(0 != (mDocDirtyState));

I'm not sure if NotifyDocumentStateChanged receiver always expects a prbool. Signature might be wrong.

PRBool nsHTMLEditor::GetNumberOfCellsInRow(nsIDOMElement* aTable, PRInt32 rowIndex) is wrong. The signature is a prbool, it returns a cell count OR an error in the same field. I'm leaving the patch as the tool produced there.
(Assignee)

Comment 1

11 years ago
Created attachment 283594 [details] [diff] [review]
This isn't meant to be applied. See comments in the bug
Attachment #283594 - Flags: review?(daniel)
(Assignee)

Comment 2

11 years ago
Created attachment 283785 [details] [diff] [review]
This isn't meant to be applied. See comments in the bug

Changed code to use !! instead of 0 != to be consistent with other modules.
Attachment #283785 - Flags: review?(daniel)
(Assignee)

Updated

11 years ago
Attachment #283594 - Attachment is obsolete: true
Attachment #283594 - Flags: review?(daniel)
(Assignee)

Comment 3

11 years ago
Daniel, 
Could you review this please?
(Assignee)

Comment 4

10 years ago
Comment on attachment 283785 [details] [diff] [review]
This isn't meant to be applied. See comments in the bug

roc, can you shed some light on the concerns mentioned int the bug?
Attachment #283785 - Flags: superreview?(roc)
(In reply to comment #0)
> These issues were found by prcheck.
> 
> I'm not sure about
> rv = mDocStateListeners[i]->NotifyDocumentStateChanged(0 != (mDocDirtyState));
> 
> I'm not sure if NotifyDocumentStateChanged receiver always expects a prbool.
> Signature might be wrong.

It does, your change is correct.

> PRBool nsHTMLEditor::GetNumberOfCellsInRow(nsIDOMElement* aTable, PRInt32
> rowIndex) is wrong. The signature is a prbool, it returns a cell count OR an
> error in the same field. I'm leaving the patch as the tool produced there.

I'd change the result to PRInt32 and return -1 on failure.
Attachment #283785 - Flags: superreview?(roc) → superreview-
Comment on attachment 283785 [details] [diff] [review]
This isn't meant to be applied. See comments in the bug

Patch needs to be updated, see comment #5

Comment 7

7 years ago
Comment on attachment 283785 [details] [diff] [review]
This isn't meant to be applied. See comments in the bug

Clearing review since these issues have been resolved through other bugs.
Attachment #283785 - Flags: review?(daniel)

Comment 8

7 years ago
Issues resolved by bug 671417 and bug 671185 .
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.