Closed Bug 1468708 Opened 6 years ago Closed 6 years ago

Rename & Move nsComposerCommands and Contoller

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla62

Tracking Flags:

Tracking

Status

firefox62

---

fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(6 files)

Bug 1468708 - Part 1. Rename nsComposerController to HTMLEditorController. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 1468708 - Part 2. Remove unimplemented commands. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 1468708 - Part 3. Clean up local functions not to calculate atom again. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 1468708 - Part 4. Use EditorBase::GetPresContext to get nsPresContext in nsComposerDocumentCommands. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 1468708 - Part 5. Rename nsComposerCommands to HTMLEditorCommands. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details
Bug 1468708 - Part 6. Move HTMLEditorCommands and HTMLEditorController to libeditor. 6 years ago Makoto Kato [:m_kato] 59 bytes, text/x-review-board-request	masayuki : review+	Details

Makoto Kato [:m_kato]

Assignee

Description

•

6 years ago

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 7

•

6 years ago

mozreview-review

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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 8

•

6 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 9

•

6 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 10

•

6 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 11

•

6 years ago

mozreview-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+

Masayuki Nakano [:masayuki] (he/him)(JST, +0900)

Comment 12

•

6 years ago

mozreview-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+

Makoto Kato [:m_kato]

Assignee

Comment 13

•

6 years ago

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

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Pulsebot

Comment 18

•

6 years ago

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

Arthur Iakab [arthur_iakab]

Comment 19

•

6 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 6 years ago

status-firefox62: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla62

Cristian Fogel [:cfogel]

Updated

•

6 years ago

Flags: qe-verify-

You need to log in before you can comment on or make changes to this bug.