1468708 - Rename & Move nsComposerCommands and Contoller

Status: RESOLVED FIXED

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

Description

[Makoto Kato [:m_kato]]

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 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 1. Rename nsComposerController to HTMLEditorController.

nsComposerController is execCommand's command controller now.  So it is better
to use mozilla::HTMLEditorController class name instead of nsComposerController.

Review commit: https://reviewboard.mozilla.org/r/250948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250948/

Comment 2

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 2. Remove unimplemented commands.

Some commands are C++ header only, so we should remove unnecessary headers.

Review commit: https://reviewboard.mozilla.org/r/250950/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250950/

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 3. Clean up local functions not to calculate atom again.

nsComposerCommands has unnecessary atom calculation, so we should remove it.
Also, since nsStyleUpdatingCommand doesn't has "all" tag name, we don't need it.

Review commit: https://reviewboard.mozilla.org/r/250952/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250952/

Comment 4

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands.

Review commit: https://reviewboard.mozilla.org/r/250954/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250954/

Comment 5

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 5. Rename nsComposer*Commands to HTMLEditor*Commands.

Rename nsComposerCommands to mozilla::*Command

Base class
- nsBaseComposerCommand -> mozilla::HTMLEditorCommandBase
- nsBaseStateUpdatingCommand -> mozilla::StateUpdatingCommandBase
- nsMultiStateCommand -> mozilla::MultiStateCommandBase

Command class
(HTMLEditorCommands)
- nsPasteNoFormattingCommand -> mozilla::PasteNoFormattingCommand
- nsStyleUpdatingCommand -> mozilla::StyleUpdatingCommand
- nsListCommand -> mozilla::ListCommand
- nsListItemCommand -> mozilla::ListItemCommand
- nsRemoveListCommand -> mozilla::RemoveListCommand
- nsIndentCommand -> mozilla::IndentCommand
- nsOutdentCommand -> mozilla::OutdentCommand
- nsParagraphStateCommand -> mozilla::ParagraphStateCommand
- nsFontFaceStateCommand -> mozilla::FontFaceStateCommand
- nsFontSizeStateCommand -> mozilla::FontSizeStateCommand
- nsFontColorStateCommand -> mozilla::FontColorStateCommand
- nsHighlightColorStateCommand -> mozilla::HighlightColorStateCommand
- nsBackgroundColorStateCommand -> mozilla::BackgroundColorStateCommand
- nsAlignCommand -> mozilla::AlignCommand
- nsAbsolutePositioningCommand -> mozilla::AbsolutePositioningCommand
- nsDecreaseZIndexCommand -> mozilla::DecreaseZIndexCommand
- nsIncreaseZIndexCommand -> mozilla::IncreaseZIndexCommand
- nsRemoveStylesCommand -> mozilla::RemoveStylesCommand
- nsIncreaseFontSizeCommand -> mozilla::IncreaseFontSizeCommand
- nsDecreaseFontSizeCommand -> mozilla::DecreaseFontSizeCommand
- nsInsertHTMLCommand -> mozilla::InsertHTMLCommand
- nsInsertTagCommand -> mozilla::InsertTagCommand

(HTMLEditorDocumentCommands)
- nsSetDocumentOptionsCommand -> mozilla::SetDocumentOptionsCommand
- nsSetDocumentStateCommand -> mozilla::SetDocumentStateCommand
- nsDocumentStateCommand -> mozilla::DocumentStateCommand

Review commit: https://reviewboard.mozilla.org/r/250956/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250956/

Comment 6

[Makoto Kato [:m_kato]]

Attachment: Bug 1468708 - Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor.

HTMLEditorController and HTMLEditorCommands is for execCommand, so we should
move from composer to libeditor since HTML editor code is in libeditor.

Review commit: https://reviewboard.mozilla.org/r/250958/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250958/

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8985331 [details]
Bug 1468708 - Part 1. Rename nsComposerController to HTMLEditorController.

https://reviewboard.mozilla.org/r/250948/#review257252

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8985332 [details]
Bug 1468708 - Part 2. Remove unimplemented commands.

https://reviewboard.mozilla.org/r/250950/#review257254

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

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 |=|.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8985334 [details]
Bug 1468708 - Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands.

https://reviewboard.mozilla.org/r/250954/#review257268

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

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 *).

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8985336 [details]
Bug 1468708 - Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor.

https://reviewboard.mozilla.org/r/250958/#review257448

Comment 13

[Makoto Kato [:m_kato]]

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.

Comment 14

[Makoto Kato [:m_kato]]

Comment on attachment 8985333 [details]
Bug 1468708 - Part 3. Clean up local functions not to calculate atom again.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250952/diff/1-2/

Comment 15

[Makoto Kato [:m_kato]]

Comment on attachment 8985334 [details]
Bug 1468708 - Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250954/diff/1-2/

Comment 16

[Makoto Kato [:m_kato]]

Comment on attachment 8985335 [details]
Bug 1468708 - Part 5. Rename nsComposer*Commands to HTMLEditor*Commands.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250956/diff/1-2/

Comment 17

[Makoto Kato [:m_kato]]

Comment on attachment 8985336 [details]
Bug 1468708 - Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250958/diff/1-2/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

Comment 19

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/eefee7582184
https://hg.mozilla.org/mozilla-central/rev/285b7c7f9df0
https://hg.mozilla.org/mozilla-central/rev/474161ab8301
https://hg.mozilla.org/mozilla-central/rev/352f3c1a4615
https://hg.mozilla.org/mozilla-central/rev/bda1559906a2
https://hg.mozilla.org/mozilla-central/rev/c1a45c216af9