A lot of classes related to editor listens to selection changes separately

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(4 attachments)

class IMEContentObserver final : public nsISelectionListener
class TextInputListener final : public nsISelectionListener
class nsComposerCommandsUpdater : public nsISelectionListener
class ResizerSelectionListener final : public nsISelectionListener
class TypeInState final : public nsISelectionListener

If EditorBase can store those instances directly, they can listen to selection changes via EditorBase without nsISelectionListener.  Fortunately, first two classes are now stored by EditorBase directly.
Comment on attachment 8945815 [details]
Bug 1433345 - part 1: Expose nsComposerCommandsUpdater with renaming it to mozilla::ComposerCOmmandsUpdater

https://reviewboard.mozilla.org/r/215892/#review221892

::: editor/composer/ComposerCommandsUpdater.h:27
(Diff revision 1)
>  class nsITransactionManager;
>  class nsPICommandUpdater;
>  
> -class nsComposerCommandsUpdater : public nsISelectionListener,
> -                                  public nsIDocumentStateListener,
> -                                  public nsITransactionListener,
> +namespace mozilla {
> +
> +class ComposerCommandsUpdater : public nsISelectionListener

class ComposerCommandsUpdater final : ...

::: editor/composer/ComposerCommandsUpdater.h:51
(Diff revision 1)
>    NS_DECL_NSITIMERCALLBACK
>  
>    // nsINamed
>    NS_DECL_NSINAMED
>  
> -  /** nsITransactionListener interfaces
> +  // nsITransactionListener

Use NS_DECL_NSITRANSACTIONLISTENER
Attachment #8945815 - Flags: review?(m_kato) → review+
Comment on attachment 8945816 [details]
Bug 1433345 - part 2: Make HTMLEditor store ComposerCommandsUpdater directly

https://reviewboard.mozilla.org/r/215894/#review221894
Attachment #8945816 - Flags: review?(m_kato) → review+
Comment on attachment 8945817 [details]
Bug 1433345 - part 3: Make HTMLEditor store ResizerSelectionListener directly

https://reviewboard.mozilla.org/r/215896/#review221920
Attachment #8945817 - Flags: review?(m_kato) → review+
Comment on attachment 8945818 [details]
Bug 1433345 - part 4: Make EditorBase derived from nsISelectionListener and notify its owning classes of selection change

https://reviewboard.mozilla.org/r/215898/#review221922

::: dom/events/IMEContentObserver.cpp:720
(Diff revision 1)
> +  if (!mIsObserving) {
> +    return;
> +  }
> +
>    int32_t count = 0;
> -  nsresult rv = aSelection->GetRangeCount(&count);
> +  nsresult rv = aSelection.GetRangeCount(&count);

Use uint32_t count = aSelection.RangeCount() to avoid checking nsresult and non-virtual function.
Attachment #8945818 - Flags: review?(m_kato) → review+
Comment on attachment 8945818 [details]
Bug 1433345 - part 4: Make EditorBase derived from nsISelectionListener and notify its owning classes of selection change

https://reviewboard.mozilla.org/r/215898/#review221922

> Use uint32_t count = aSelection.RangeCount() to avoid checking nsresult and non-virtual function.

s/non-virtual/virtual/
Priority: -- → P3
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d0cd3d4c1a04
part 1: Expose nsComposerCommandsUpdater with renaming it to mozilla::ComposerCOmmandsUpdater r=m_kato
https://hg.mozilla.org/integration/autoland/rev/510153b3ba8d
part 2: Make HTMLEditor store ComposerCommandsUpdater directly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f2d590678c07
part 3: Make HTMLEditor store ResizerSelectionListener directly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/1bb95c1b49c8
part 4: Make EditorBase derived from nsISelectionListener and notify its owning classes of selection change r=m_kato
Depends on: 1437795
You need to log in before you can comment on or make changes to this bug.