Skip to content

Commit 2773c4d

Browse files
masayuki-nakanomasayuki-nakano
committed
Bug 1310618 Replace nsresult variables |res| with |rv| under editor/ r=Ehsan
In our coding rules, variable names of nsresult should be rv. Indeed, when you see |rv| in the code, you must assume that its type if nsresult. However, a lot of code under editor/ uses |res| for the variables of nsresult. Let's replace |res| with |rv|. And this patch improves following points: 1. When |rv| is set in both |if| and |else| block and they are check outside of them, this moves the check into each |if| and |else| block because even if the failure is notified with warning, you cannot see which case was performed and failed. This change makes it clear. 2. When |return rv;| returns non-error code because |rv| is checked with NS_ENSURE_SUCCESS() immediately before, setting replacing it with |return NS_OK;| is clearer. 3. Move declaration of |nsresult rv| into smaller scope as far as possible. This prevents setting rv to unexpected value and easier to check its value at reading the code. MozReview-Commit-ID: 9MAqj7sFey3 --HG-- extra : rebase_source : 0fd316b851ea616b3a95d8c1afc111ff55e11993
1 parent 701f3b7 commit 2773c4d

21 files changed

+3036
-2782
lines changed

‎editor/composer/nsComposerCommands.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,8 +1057,8 @@ nsDecreaseZIndexCommand::IsCommandEnabled(const char * aCommandName,
10571057
*outCmdEnabled = false;
10581058
if (positionedElement) {
10591059
int32_t z;
1060-
nsresult res = htmlEditor->GetElementZIndex(positionedElement, &z);
1061-
NS_ENSURE_SUCCESS(res, res);
1060+
nsresult rv = htmlEditor->GetElementZIndex(positionedElement, &z);
1061+
NS_ENSURE_SUCCESS(rv, rv);
10621062
*outCmdEnabled = (z > 0);
10631063
}
10641064

‎editor/libeditor/CSSEditUtils.cpp

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,8 @@ CSSEditUtils::RemoveCSSInlineStyle(nsIDOMNode* aNode,
589589
NS_ENSURE_STATE(element);
590590

591591
// remove the property from the style attribute
592-
nsresult res = RemoveCSSProperty(*element, *aProperty, aPropertyValue);
593-
NS_ENSURE_SUCCESS(res, res);
592+
nsresult rv = RemoveCSSProperty(*element, *aProperty, aPropertyValue);
593+
NS_ENSURE_SUCCESS(rv, rv);
594594

595595
if (!element->IsHTMLElement(nsGkAtoms::span) ||
596596
HTMLEditor::HasAttributes(element)) {
@@ -903,12 +903,12 @@ CSSEditUtils::SetCSSEquivalentToHTMLStyle(Element* aElement,
903903
// something is pretty badly wrong. In this case we assert so that hopefully
904904
// someone will notice, but there's nothing more sensible to do than just
905905
// return the count and carry on.
906-
nsresult res = SetCSSEquivalentToHTMLStyle(aElement->AsDOMNode(),
907-
aProperty, aAttribute,
908-
aValue, &count,
909-
aSuppressTransaction);
910-
NS_ASSERTION(NS_SUCCEEDED(res), "SetCSSEquivalentToHTMLStyle failed");
911-
NS_ENSURE_SUCCESS(res, count);
906+
nsresult rv = SetCSSEquivalentToHTMLStyle(aElement->AsDOMNode(),
907+
aProperty, aAttribute,
908+
aValue, &count,
909+
aSuppressTransaction);
910+
NS_ASSERTION(NS_SUCCEEDED(rv), "SetCSSEquivalentToHTMLStyle failed");
911+
NS_ENSURE_SUCCESS(rv, count);
912912
return count;
913913
}
914914

@@ -939,9 +939,9 @@ CSSEditUtils::SetCSSEquivalentToHTMLStyle(nsIDOMNode* aNode,
939939
// set the individual CSS inline styles
940940
*aCount = cssPropertyArray.Length();
941941
for (int32_t index = 0; index < *aCount; index++) {
942-
nsresult res = SetCSSProperty(*element, *cssPropertyArray[index],
943-
cssValueArray[index], aSuppressTransaction);
944-
NS_ENSURE_SUCCESS(res, res);
942+
nsresult rv = SetCSSProperty(*element, *cssPropertyArray[index],
943+
cssValueArray[index], aSuppressTransaction);
944+
NS_ENSURE_SUCCESS(rv, rv);
945945
}
946946
return NS_OK;
947947
}
@@ -987,11 +987,11 @@ CSSEditUtils::RemoveCSSEquivalentToHTMLStyle(Element* aElement,
987987
// remove the individual CSS inline styles
988988
int32_t count = cssPropertyArray.Length();
989989
for (int32_t index = 0; index < count; index++) {
990-
nsresult res = RemoveCSSProperty(*aElement,
991-
*cssPropertyArray[index],
992-
cssValueArray[index],
993-
aSuppressTransaction);
994-
NS_ENSURE_SUCCESS(res, res);
990+
nsresult rv = RemoveCSSProperty(*aElement,
991+
*cssPropertyArray[index],
992+
cssValueArray[index],
993+
aSuppressTransaction);
994+
NS_ENSURE_SUCCESS(rv, rv);
995995
}
996996
return NS_OK;
997997
}
@@ -1026,9 +1026,9 @@ CSSEditUtils::GetCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
10261026
for (int32_t index = 0; index < count; index++) {
10271027
nsAutoString valueString;
10281028
// retrieve the specified/computed value of the property
1029-
nsresult res = GetCSSInlinePropertyBase(theElement, cssPropertyArray[index],
1030-
valueString, aStyleType);
1031-
NS_ENSURE_SUCCESS(res, res);
1029+
nsresult rv = GetCSSInlinePropertyBase(theElement, cssPropertyArray[index],
1030+
valueString, aStyleType);
1031+
NS_ENSURE_SUCCESS(rv, rv);
10321032
// append the value to aValueString (possibly with a leading whitespace)
10331033
if (index) {
10341034
aValueString.Append(char16_t(' '));
@@ -1067,10 +1067,10 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(nsINode* aNode,
10671067
{
10681068
MOZ_ASSERT(aNode && aProperty);
10691069
bool isSet;
1070-
nsresult res = IsCSSEquivalentToHTMLInlineStyleSet(aNode->AsDOMNode(),
1071-
aProperty, aAttribute,
1072-
isSet, aValue, aStyleType);
1073-
NS_ENSURE_SUCCESS(res, false);
1070+
nsresult rv = IsCSSEquivalentToHTMLInlineStyleSet(aNode->AsDOMNode(),
1071+
aProperty, aAttribute,
1072+
isSet, aValue, aStyleType);
1073+
NS_ENSURE_SUCCESS(rv, false);
10741074
return isSet;
10751075
}
10761076

@@ -1091,9 +1091,10 @@ CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
10911091
do {
10921092
valueString.Assign(htmlValueString);
10931093
// get the value of the CSS equivalent styles
1094-
nsresult res = GetCSSEquivalentToHTMLInlineStyleSet(node, aHTMLProperty, aHTMLAttribute,
1095-
valueString, aStyleType);
1096-
NS_ENSURE_SUCCESS(res, res);
1094+
nsresult rv =
1095+
GetCSSEquivalentToHTMLInlineStyleSet(node, aHTMLProperty, aHTMLAttribute,
1096+
valueString, aStyleType);
1097+
NS_ENSURE_SUCCESS(rv, rv);
10971098

10981099
// early way out if we can
10991100
if (valueString.IsEmpty()) {
@@ -1398,8 +1399,10 @@ CSSEditUtils::SetCSSProperty(nsIDOMElement* aElement,
13981399
{
13991400
nsCOMPtr<nsIDOMCSSStyleDeclaration> cssDecl;
14001401
uint32_t length;
1401-
nsresult res = GetInlineStyles(aElement, getter_AddRefs(cssDecl), &length);
1402-
if (NS_FAILED(res) || !cssDecl) return res;
1402+
nsresult rv = GetInlineStyles(aElement, getter_AddRefs(cssDecl), &length);
1403+
if (NS_FAILED(rv) || !cssDecl) {
1404+
return rv;
1405+
}
14031406

14041407
return cssDecl->SetProperty(aProperty,
14051408
aValue,

‎editor/libeditor/CompositionTransaction.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,21 @@ CompositionTransaction::DoTransaction()
6262
NS_ENSURE_TRUE(selCon, NS_ERROR_NOT_INITIALIZED);
6363

6464
// Advance caret: This requires the presentation shell to get the selection.
65-
nsresult res;
6665
if (mReplaceLength == 0) {
67-
res = mTextNode->InsertData(mOffset, mStringToInsert);
66+
nsresult rv = mTextNode->InsertData(mOffset, mStringToInsert);
67+
if (NS_WARN_IF(NS_FAILED(rv))) {
68+
return rv;
69+
}
6870
} else {
69-
res = mTextNode->ReplaceData(mOffset, mReplaceLength, mStringToInsert);
71+
nsresult rv =
72+
mTextNode->ReplaceData(mOffset, mReplaceLength, mStringToInsert);
73+
if (NS_WARN_IF(NS_FAILED(rv))) {
74+
return rv;
75+
}
7076
}
71-
NS_ENSURE_SUCCESS(res, res);
7277

73-
res = SetSelectionForRanges();
74-
NS_ENSURE_SUCCESS(res, res);
78+
nsresult rv = SetSelectionForRanges();
79+
NS_ENSURE_SUCCESS(rv, rv);
7580

7681
return NS_OK;
7782
}
@@ -84,14 +89,14 @@ CompositionTransaction::UndoTransaction()
8489
RefPtr<Selection> selection = mEditorBase.GetSelection();
8590
NS_ENSURE_TRUE(selection, NS_ERROR_NOT_INITIALIZED);
8691

87-
nsresult res = mTextNode->DeleteData(mOffset, mStringToInsert.Length());
88-
NS_ENSURE_SUCCESS(res, res);
92+
nsresult rv = mTextNode->DeleteData(mOffset, mStringToInsert.Length());
93+
NS_ENSURE_SUCCESS(rv, rv);
8994

9095
// set the selection to the insertion point where the string was removed
91-
res = selection->Collapse(mTextNode, mOffset);
92-
NS_ASSERTION(NS_SUCCEEDED(res),
96+
rv = selection->Collapse(mTextNode, mOffset);
97+
NS_ASSERTION(NS_SUCCEEDED(rv),
9398
"Selection could not be collapsed after undo of IME insert.");
94-
NS_ENSURE_SUCCESS(res, res);
99+
NS_ENSURE_SUCCESS(rv, rv);
95100

96101
return NS_OK;
97102
}

‎editor/libeditor/DeleteRangeTransaction.cpp

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ NS_IMETHODIMP
6363
DeleteRangeTransaction::DoTransaction()
6464
{
6565
MOZ_ASSERT(mRange && mEditorBase);
66-
nsresult res;
6766

6867
// build the child transactions
6968
nsCOMPtr<nsINode> startParent = mRange->GetStartParent();
@@ -74,33 +73,35 @@ DeleteRangeTransaction::DoTransaction()
7473

7574
if (startParent == endParent) {
7675
// the selection begins and ends in the same node
77-
res = CreateTxnsToDeleteBetween(startParent, startOffset, endOffset);
78-
NS_ENSURE_SUCCESS(res, res);
76+
nsresult rv =
77+
CreateTxnsToDeleteBetween(startParent, startOffset, endOffset);
78+
NS_ENSURE_SUCCESS(rv, rv);
7979
} else {
8080
// the selection ends in a different node from where it started. delete
8181
// the relevant content in the start node
82-
res = CreateTxnsToDeleteContent(startParent, startOffset, nsIEditor::eNext);
83-
NS_ENSURE_SUCCESS(res, res);
82+
nsresult rv =
83+
CreateTxnsToDeleteContent(startParent, startOffset, nsIEditor::eNext);
84+
NS_ENSURE_SUCCESS(rv, rv);
8485
// delete the intervening nodes
85-
res = CreateTxnsToDeleteNodesBetween();
86-
NS_ENSURE_SUCCESS(res, res);
86+
rv = CreateTxnsToDeleteNodesBetween();
87+
NS_ENSURE_SUCCESS(rv, rv);
8788
// delete the relevant content in the end node
88-
res = CreateTxnsToDeleteContent(endParent, endOffset, nsIEditor::ePrevious);
89-
NS_ENSURE_SUCCESS(res, res);
89+
rv = CreateTxnsToDeleteContent(endParent, endOffset, nsIEditor::ePrevious);
90+
NS_ENSURE_SUCCESS(rv, rv);
9091
}
9192

9293
// if we've successfully built this aggregate transaction, then do it.
93-
res = EditAggregateTransaction::DoTransaction();
94-
NS_ENSURE_SUCCESS(res, res);
94+
nsresult rv = EditAggregateTransaction::DoTransaction();
95+
NS_ENSURE_SUCCESS(rv, rv);
9596

9697
// only set selection to deletion point if editor gives permission
9798
bool bAdjustSelection;
9899
mEditorBase->ShouldTxnSetSelection(&bAdjustSelection);
99100
if (bAdjustSelection) {
100101
RefPtr<Selection> selection = mEditorBase->GetSelection();
101102
NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
102-
res = selection->Collapse(startParent, startOffset);
103-
NS_ENSURE_SUCCESS(res, res);
103+
rv = selection->Collapse(startParent, startOffset);
104+
NS_ENSURE_SUCCESS(rv, rv);
104105
}
105106
// else do nothing - dom range gravity will adjust selection
106107

@@ -152,8 +153,8 @@ DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
152153
new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
153154
numToDel, mRangeUpdater);
154155

155-
nsresult res = transaction->Init();
156-
NS_ENSURE_SUCCESS(res, res);
156+
nsresult rv = transaction->Init();
157+
NS_ENSURE_SUCCESS(rv, rv);
157158

158159
AppendChild(transaction);
159160
return NS_OK;
@@ -162,18 +163,20 @@ DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
162163
nsCOMPtr<nsIContent> child = aNode->GetChildAt(aStartOffset);
163164
NS_ENSURE_STATE(child);
164165

165-
nsresult res = NS_OK;
166+
// XXX This looks odd. Only when the last transaction causes error at
167+
// calling Init(), the result becomes error. Otherwise, always NS_OK.
168+
nsresult rv = NS_OK;
166169
for (int32_t i = aStartOffset; i < aEndOffset; ++i) {
167170
RefPtr<DeleteNodeTransaction> transaction = new DeleteNodeTransaction();
168-
res = transaction->Init(mEditorBase, child, mRangeUpdater);
169-
if (NS_SUCCEEDED(res)) {
171+
rv = transaction->Init(mEditorBase, child, mRangeUpdater);
172+
if (NS_SUCCEEDED(rv)) {
170173
AppendChild(transaction);
171174
}
172175

173176
child = child->GetNextSibling();
174177
}
175178

176-
NS_ENSURE_SUCCESS(res, res);
179+
NS_ENSURE_SUCCESS(rv, rv);
177180
return NS_OK;
178181
}
179182

@@ -201,8 +204,8 @@ DeleteRangeTransaction::CreateTxnsToDeleteContent(nsINode* aNode,
201204
new DeleteTextTransaction(*mEditorBase, *dataNode, start, numToDelete,
202205
mRangeUpdater);
203206

204-
nsresult res = transaction->Init();
205-
NS_ENSURE_SUCCESS(res, res);
207+
nsresult rv = transaction->Init();
208+
NS_ENSURE_SUCCESS(rv, rv);
206209

207210
AppendChild(transaction);
208211
}
@@ -216,16 +219,16 @@ DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween()
216219
{
217220
nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
218221

219-
nsresult res = iter->Init(mRange);
220-
NS_ENSURE_SUCCESS(res, res);
222+
nsresult rv = iter->Init(mRange);
223+
NS_ENSURE_SUCCESS(rv, rv);
221224

222225
while (!iter->IsDone()) {
223226
nsCOMPtr<nsINode> node = iter->GetCurrentNode();
224227
NS_ENSURE_TRUE(node, NS_ERROR_NULL_POINTER);
225228

226229
RefPtr<DeleteNodeTransaction> transaction = new DeleteNodeTransaction();
227-
res = transaction->Init(mEditorBase, node, mRangeUpdater);
228-
NS_ENSURE_SUCCESS(res, res);
230+
rv = transaction->Init(mEditorBase, node, mRangeUpdater);
231+
NS_ENSURE_SUCCESS(rv, rv);
229232
AppendChild(transaction);
230233

231234
iter->Next();

‎editor/libeditor/DeleteTextTransaction.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ DeleteTextTransaction::DoTransaction()
5858
MOZ_ASSERT(mCharData);
5959

6060
// Get the text that we're about to delete
61-
nsresult res = mCharData->SubstringData(mOffset, mNumCharsToDelete,
62-
mDeletedText);
63-
MOZ_ASSERT(NS_SUCCEEDED(res));
64-
res = mCharData->DeleteData(mOffset, mNumCharsToDelete);
65-
NS_ENSURE_SUCCESS(res, res);
61+
nsresult rv = mCharData->SubstringData(mOffset, mNumCharsToDelete,
62+
mDeletedText);
63+
MOZ_ASSERT(NS_SUCCEEDED(rv));
64+
rv = mCharData->DeleteData(mOffset, mNumCharsToDelete);
65+
NS_ENSURE_SUCCESS(rv, rv);
6666

6767
if (mRangeUpdater) {
6868
mRangeUpdater->SelAdjDeleteText(mCharData, mOffset, mNumCharsToDelete);
@@ -72,10 +72,10 @@ DeleteTextTransaction::DoTransaction()
7272
if (mEditorBase.GetShouldTxnSetSelection()) {
7373
RefPtr<Selection> selection = mEditorBase.GetSelection();
7474
NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
75-
res = selection->Collapse(mCharData, mOffset);
76-
NS_ASSERTION(NS_SUCCEEDED(res),
75+
rv = selection->Collapse(mCharData, mOffset);
76+
NS_ASSERTION(NS_SUCCEEDED(rv),
7777
"Selection could not be collapsed after undo of deletetext");
78-
NS_ENSURE_SUCCESS(res, res);
78+
NS_ENSURE_SUCCESS(rv, rv);
7979
}
8080
// Else do nothing - DOM Range gravity will adjust selection
8181
return NS_OK;

0 commit comments

Comments
 (0)