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

RESOLVED FIXED in Firefox 63

Status

()

Product:
Core
Component:
Editor
Type:
enhancement
Status:
RESOLVED FIXED
Reported:
10 months ago
Modified:
10 months ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Version:
Trunk
Target:
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

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
: review+
Details
Assignee

Description

•
10 months ago 
I forgot to update HTMLEditorDocumentCommands.cpp when I work on bug 1450882.
Comment hidden (mozreview-request)

Comment 4

•
10 months ago
mozreview-review 
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+
Assignee

Comment 5

•
10 months ago 
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)

Comment 6

•
10 months ago 
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)

Comment 7

•
10 months ago 
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

Comment 8

•
10 months ago
bugherder 
https://hg.mozilla.org/mozilla-central/rev/cfb544de8a72
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months 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.