Closed Bug 1476294 Opened 4 years ago Closed 4 years ago

Make HTMLEditorDocumentCommands.cpp use new non-virtual methods of nsCommandParams

Categories

(Core :: DOM: Editor, enhancement)

Product:
Core
Component:
DOM: Editor
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(1 file)

Bug 1476294 - Make HTMLEditorDocumentCommands.cpp use non-virtual methods of nsCommandParams instead of virtual methods of nsICommandParams
59 bytes, text/x-review-board-request
ehsan.akhgari
: review+
Details 
I forgot to update HTMLEditorDocumentCommands.cpp when I work on bug 1450882.
Comment on attachment 8992858 [details]
Bug 1476294 - Make HTMLEditorDocumentCommands.cpp use non-virtual methods of nsCommandParams instead of virtual methods of nsICommandParams

https://reviewboard.mozilla.org/r/257704/#review264692

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:242
(Diff revision 1)
> +      if (NS_WARN_IF(NS_FAILED(rv))) {
> +        return rv;
> -    }
> +      }
> -
> -    return textEditor->ResetModificationCount();
> +      return NS_OK;
> +    }
> +    nsresult rv = textEditor->ResetModificationCount();

You could just return ResetModificationCount() directly here.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:268
(Diff revision 1)
> +    nsresult rv =
>        textEditor->RemoveFlags(nsIPlaintextEditor::eEditorReadonlyMask);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

Here also, you could directly return the functions you're calling.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:285
(Diff revision 1)
> -
> -    return htmlEditor->SetIsCSSEnabled(desireCSS);
> +    }
> +    nsresult rv = htmlEditor->SetIsCSSEnabled(desireCSS);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

Here too.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:303
(Diff revision 1)
> -
> -    return htmlEditor->SetReturnInParagraphCreatesNewParagraph(!insertBrOnReturn);
> +    nsresult rv =
> +      htmlEditor->SetReturnInParagraphCreatesNewParagraph(!insertBrOnReturn);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

And here.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:351
(Diff revision 1)
> -
> -    return htmlEditor->SetObjectResizingEnabled(enabled);
> +    }
> +    nsresult rv = htmlEditor->SetObjectResizingEnabled(enabled);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

And here.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:368
(Diff revision 1)
> -
> -    return htmlEditor->SetInlineTableEditingEnabled(enabled);
> +    }
> +    nsresult rv = htmlEditor->SetInlineTableEditingEnabled(enabled);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

Ditto.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:411
(Diff revision 1)
> -    return aParams->SetBooleanValue(STATE_ATTRIBUTE, modified);
> +    }
> +    rv = params->SetBool(STATE_ATTRIBUTE, modified);
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

Ditto.

::: editor/libeditor/HTMLEditorDocumentCommands.cpp:419
(Diff revision 1)
>    if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentReadOnly")) {
> -    NS_ENSURE_ARG_POINTER(aParams);
> -    return aParams->SetBooleanValue(STATE_ATTRIBUTE, textEditor->IsReadonly());
> +    rv = params->SetBool(STATE_ATTRIBUTE, textEditor->IsReadonly());
> +    if (NS_WARN_IF(NS_FAILED(rv))) {
> +      return rv;
> +    }
> +    return NS_OK;

And so on, there seem to be many more.  :-)
Attachment #8992858 - Flags: review?(ehsan) → review+
Of course, yes, they are. But I'd like to keep the style since they put warnings on each failure when I run debug build. That may help me to investigate where fails do something on reported bug.

However, if they cause increasing binary size a lot or something bad side effect which is not better than the merit, I don't mind to rewrite them, though.
Flags: needinfo?(ehsan)
Oh, no problem, I just didn't realize that the addition of warnings is the intention, there was nothing to indicate that.  :-)  Sorry for the miscommunication.  You can land the patch as is.
Flags: needinfo?(ehsan)
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/cfb544de8a72
Make HTMLEditorDocumentCommands.cpp use non-virtual methods of nsCommandParams instead of virtual methods of nsICommandParams r=Ehsan
https://hg.mozilla.org/mozilla-central/rev/cfb544de8a72
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.