Closed Bug 1468708 Opened 4 years ago Closed 4 years ago

Rename & Move nsComposerCommands and Contoller

Categories

(Core :: DOM: Editor, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(6 files)

nsComposerCommands was the menu implementation of HTML editor application "Composer".  But now we use it for execCommand, so we should rename another name such as HTMLEditorCommands and move this to libeditor
Comment on attachment 8985331 [details]
Bug 1468708 - Part 1. Rename nsComposerController to HTMLEditorController.

https://reviewboard.mozilla.org/r/250948/#review257252
Attachment #8985331 - Flags: review?(masayuki) → review+
Comment on attachment 8985332 [details]
Bug 1468708 - Part 2. Remove unimplemented commands.

https://reviewboard.mozilla.org/r/250950/#review257254
Attachment #8985332 - Flags: review?(masayuki) → review+
Comment on attachment 8985333 [details]
Bug 1468708 - Part 3. Clean up local functions not to calculate atom again.

https://reviewboard.mozilla.org/r/250952/#review257258

::: commit-message-1b0bc:4
(Diff revision 1)
> +Bug 1468708 - Part 3. Clean up local functions not to calculate atom again. r?masayuki
> +
> +nsComposerCommands has unnecessary atom calculation, so we should remove it.
> +Also, since nsStyleUpdatingCommand doesn't has "all" tag name, we don't need it.

s/doesn't has/doesn't have
s/all tag name/all tag names
s/need it/need to support it?

::: editor/composer/nsComposerCommands.cpp:280
(Diff revision 1)
> -    nsDependentAtomString tagName(mTagName);
> -    if (mTagName == nsGkAtoms::sub || mTagName == nsGkAtoms::sup) {
> +  if (mTagName == nsGkAtoms::sub || mTagName == nsGkAtoms::sup) {
> -      rv = RemoveTextProperty(aHTMLEditor, tagName);
> +    rv = aHTMLEditor->RemoveInlineProperty(mTagName, nullptr);
> +  }
> +  if (NS_SUCCEEDED(rv)) {
> +    rv =aHTMLEditor->SetInlineProperty(mTagName, nullptr, EmptyString());

Insert a space after |=|.
Attachment #8985333 - Flags: review?(masayuki) → review+
Comment on attachment 8985334 [details]
Bug 1468708 - Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands.

https://reviewboard.mozilla.org/r/250954/#review257268
Attachment #8985334 - Flags: review?(masayuki) → review+
Comment on attachment 8985335 [details]
Bug 1468708 - Part 5. Rename nsComposer*Commands to HTMLEditor*Commands.

https://reviewboard.mozilla.org/r/250956/#review257274

::: editor/composer/HTMLEditorCommands.h:42
(Diff revision 1)
> -  NS_IMETHOD IsCommandEnabled(const char * aCommandName, nsISupports *aCommandRefCon, bool *_retval) override = 0;
> -  NS_IMETHOD DoCommand(const char * aCommandName, nsISupports *aCommandRefCon) override = 0;
> +  NS_IMETHOD IsCommandEnabled(const char* aCommandName,
> +                              nsISupports* aCommandRefCon,
> +                              bool* _retval) override = 0;
> +  NS_IMETHOD DoCommand(const char* aCommandName,
> +                       nsISupports* aCommandRefCon) override = 0;

They are overriding methods but |= 0|... Do we really need those decls?

::: editor/composer/HTMLEditorCommands.cpp:60
(Diff revision 1)
>  
> -NS_IMPL_ISUPPORTS(nsBaseComposerCommand, nsIControllerCommand)
> +NS_IMPL_ISUPPORTS(HTMLEditorCommandBase, nsIControllerCommand)
>  
>  
> -nsBaseStateUpdatingCommand::nsBaseStateUpdatingCommand(nsAtom* aTagName)
> -: nsBaseComposerCommand()
> +StateUpdatingCommandBase::StateUpdatingCommandBase(nsAtom* aTagName)
> +: HTMLEditorCommandBase()

indent before ":".

::: editor/composer/HTMLEditorCommands.cpp:61
(Diff revision 1)
> -NS_IMPL_ISUPPORTS(nsBaseComposerCommand, nsIControllerCommand)
> +NS_IMPL_ISUPPORTS(HTMLEditorCommandBase, nsIControllerCommand)
>  
>  
> -nsBaseStateUpdatingCommand::nsBaseStateUpdatingCommand(nsAtom* aTagName)
> -: nsBaseComposerCommand()
> +StateUpdatingCommandBase::StateUpdatingCommandBase(nsAtom* aTagName)
> +: HTMLEditorCommandBase()
>  , mTagName(aTagName)

And here.

::: editor/composer/HTMLEditorCommands.cpp:187
(Diff revision 1)
>  
>    return aParams->SetBooleanValue(STATE_ENABLED, enabled);
>  }
>  
> -nsStyleUpdatingCommand::nsStyleUpdatingCommand(nsAtom* aTagName)
> -: nsBaseStateUpdatingCommand(aTagName)
> +StyleUpdatingCommand::StyleUpdatingCommand(nsAtom* aTagName)
> +: StateUpdatingCommandBase(aTagName)

indent.

::: editor/composer/HTMLEditorCommands.cpp:288
(Diff revision 1)
>  
>    return rv;
>  }
>  
> -nsListCommand::nsListCommand(nsAtom* aTagName)
> -: nsBaseStateUpdatingCommand(aTagName)
> +ListCommand::ListCommand(nsAtom* aTagName)
> +: StateUpdatingCommandBase(aTagName)

indent.

::: editor/composer/HTMLEditorCommands.cpp:343
(Diff revision 1)
>  
>    return rv;
>  }
>  
> -nsListItemCommand::nsListItemCommand(nsAtom* aTagName)
> -: nsBaseStateUpdatingCommand(aTagName)
> +ListItemCommand::ListItemCommand(nsAtom* aTagName)
> +: StateUpdatingCommandBase(aTagName)

indent.

::: editor/composer/HTMLEditorCommands.cpp:1418
(Diff revision 1)
>    IsCommandEnabled(aCommandName, refCon, &outCmdEnabled);
>    return aParams->SetBooleanValue(STATE_ENABLED, outCmdEnabled);
>  }
>  
> -nsInsertTagCommand::nsInsertTagCommand(nsAtom* aTagName)
> -: nsBaseComposerCommand()
> +InsertTagCommand::InsertTagCommand(nsAtom* aTagName)
> +: HTMLEditorCommandBase()

indent.

::: editor/composer/HTMLEditorCommands.cpp:1419
(Diff revision 1)
>    return aParams->SetBooleanValue(STATE_ENABLED, outCmdEnabled);
>  }
>  
> -nsInsertTagCommand::nsInsertTagCommand(nsAtom* aTagName)
> -: nsBaseComposerCommand()
> +InsertTagCommand::InsertTagCommand(nsAtom* aTagName)
> +: HTMLEditorCommandBase()
>  , mTagName(aTagName)

And here.

::: editor/composer/HTMLEditorDocumentCommands.cpp:39
(Diff revision 1)
>  #define STATE_DATA "state_data"
>  
> +namespace mozilla {
>  
>  NS_IMETHODIMP
> -nsSetDocumentOptionsCommand::IsCommandEnabled(const char * aCommandName,
> +SetDocumentOptionsCommand::IsCommandEnabled(const char * aCommandName,

const char* (position of *)

::: editor/composer/HTMLEditorDocumentCommands.cpp:56
(Diff revision 1)
>    *outCmdEnabled = textEditor->IsSelectionEditable();
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentOptionsCommand::DoCommand(const char *aCommandName,
> +SetDocumentOptionsCommand::DoCommand(const char *aCommandName,

const char* (position of *)

::: editor/composer/HTMLEditorDocumentCommands.cpp:63
(Diff revision 1)
>  {
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentOptionsCommand::DoCommandParams(const char *aCommandName,
> +SetDocumentOptionsCommand::DoCommandParams(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:103
(Diff revision 1)
>  
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentOptionsCommand::GetCommandStateParams(const char *aCommandName,
> +SetDocumentOptionsCommand::GetCommandStateParams(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:165
(Diff revision 1)
>   *    for more than one of this type of command
>   *    We check the input command param for different behavior
>   */
>  
>  NS_IMETHODIMP
> -nsSetDocumentStateCommand::IsCommandEnabled(const char * aCommandName,
> +SetDocumentStateCommand::IsCommandEnabled(const char * aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:176
(Diff revision 1)
>    *outCmdEnabled = true;
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentStateCommand::DoCommand(const char *aCommandName,
> +SetDocumentStateCommand::DoCommand(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:183
(Diff revision 1)
>  {
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentStateCommand::DoCommandParams(const char *aCommandName,
> +SetDocumentStateCommand::DoCommandParams(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:317
(Diff revision 1)
>  
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
>  NS_IMETHODIMP
> -nsSetDocumentStateCommand::GetCommandStateParams(const char *aCommandName,
> +SetDocumentStateCommand::GetCommandStateParams(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:468
(Diff revision 1)
>   *     "state_data" param's value
>   *
>   */
>  
>  NS_IMETHODIMP
> -nsDocumentStateCommand::IsCommandEnabled(const char* aCommandName,
> +DocumentStateCommand::IsCommandEnabled(const char* aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:479
(Diff revision 1)
>    *outCmdEnabled = false;
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> -nsDocumentStateCommand::DoCommand(const char *aCommandName,
> +DocumentStateCommand::DoCommand(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:486
(Diff revision 1)
>  {
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
>  NS_IMETHODIMP
> -nsDocumentStateCommand::DoCommandParams(const char *aCommandName,
> +DocumentStateCommand::DoCommandParams(const char *aCommandName,

const char* (position of *).

::: editor/composer/HTMLEditorDocumentCommands.cpp:494
(Diff revision 1)
>  {
>    return NS_ERROR_NOT_IMPLEMENTED;
>  }
>  
>  NS_IMETHODIMP
> -nsDocumentStateCommand::GetCommandStateParams(const char *aCommandName,
> +DocumentStateCommand::GetCommandStateParams(const char *aCommandName,

const char* (position of *).
Attachment #8985335 - Flags: review?(masayuki) → review+
Comment on attachment 8985336 [details]
Bug 1468708 - Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor.

https://reviewboard.mozilla.org/r/250958/#review257448
Attachment #8985336 - Flags: review?(masayuki) → review+
Comment on attachment 8985335 [details]
Bug 1468708 - Part 5. Rename nsComposer*Commands to HTMLEditor*Commands.

https://reviewboard.mozilla.org/r/250956/#review257480

::: editor/composer/HTMLEditorCommands.h:42
(Diff revision 1)
> -  NS_IMETHOD IsCommandEnabled(const char * aCommandName, nsISupports *aCommandRefCon, bool *_retval) override = 0;
> -  NS_IMETHOD DoCommand(const char * aCommandName, nsISupports *aCommandRefCon) override = 0;
> +  NS_IMETHOD IsCommandEnabled(const char* aCommandName,
> +                              nsISupports* aCommandRefCon,
> +                              bool* _retval) override = 0;
> +  NS_IMETHOD DoCommand(const char* aCommandName,
> +                       nsISupports* aCommandRefCon) override = 0;

Ah, unnecessary.  I should remove this.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/eefee7582184
Part 1. Rename nsComposerController to HTMLEditorController. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/285b7c7f9df0
Part 2. Remove unimplemented commands. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/474161ab8301
Part 3. Clean up local functions not to calculate atom again. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/352f3c1a4615
Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/bda1559906a2
Part 5. Rename nsComposer*Commands to HTMLEditor*Commands. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/c1a45c216af9
Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor. r=masayuki
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.