Closed Bug 1463327 Opened 2 years ago Closed 2 years ago

Reorganize scope of methods of EditorBase, TextEditor and HTMLEditor

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Currently, some public methods but not inherited from nsI*Editor may be referred from:

- classes handling comment
- *EditRules class
- *Transaction class
- WSRunObject class

Actual external call is only the first case. And some of those classes are already friend of the editor classes.

At implementing InputEvent.inputType, editor classes need to store top level input type when public methods called from external.

Therefore, for making editor classes safer, public methods which are called only by internal classes of editor should be private or protected but they should be explicitly documented which can be called by internal classes.
Merged the change of bug 1463330.
Depends on: 1463330
Comment on attachment 8979925 [details]
Bug 1463327 - part 1: Change scope of some methods of EditorBase which won't be called by non-helper classes of editing to protected

https://reviewboard.mozilla.org/r/246116/#review252422

::: editor/libeditor/HTMLStyleEditor.cpp:1054
(Diff revision 1)
>          break;
>        }
>  
>        // just ignore any non-editable nodes
>        if (content->GetAsText() && (!IsEditable(content) ||
> -                                   IsEmptyTextNode(this, content))) {
> +                                   IsEmptyTextNode(*content))) {

Since GetAsText is true, this can replace IsEmptyTextNode with IsEmptyNode.
Attachment #8979925 - Flags: review?(m_kato) → review+
Comment on attachment 8979925 [details]
Bug 1463327 - part 1: Change scope of some methods of EditorBase which won't be called by non-helper classes of editing to protected

https://reviewboard.mozilla.org/r/246116/#review252422

> Since GetAsText is true, this can replace IsEmptyTextNode with IsEmptyNode.

Hmm, yes, but IsEmptyNode() requires out param. Perhaps, we should sort out around IsEmptyNode() first (bug 1359404).
Comment on attachment 8979926 [details]
Bug 1463327 - part 2: Change scope of some methods of TextEditor which won't be called by non-helper classes of editing to protected

https://reviewboard.mozilla.org/r/246118/#review252460
Attachment #8979926 - Flags: review?(m_kato) → review+
Comment on attachment 8979927 [details]
Bug 1463327 - part 3: Change scope of some methods of HTMLEditor which won't be called by non-helper classes of editing to protected

https://reviewboard.mozilla.org/r/246120/#review252424

::: editor/libeditor/HTMLEditor.h:116
(Diff revision 1)
> +
> +  bool GetReturnInParagraphCreatesNewParagraph();
> +  Element* GetSelectionContainer();
> +
> +  // nsISelectionListener overrides
> +  NS_DECL_NSISELECTIONLISTENER

Why doesn't NS_DECL_NSISELECTIONLISTENER move to previous HTMLEditor() define?

::: editor/libeditor/HTMLEditor.h:120
(Diff revision 1)
> +  // nsISelectionListener overrides
> +  NS_DECL_NSISELECTIONLISTENER
>  
> -  // Miscellaneous
> +  // nsICSSLoaderObserver
> +  NS_IMETHOD StyleSheetLoaded(StyleSheet* aSheet,
> +                              bool aWasAlternate, nsresult aStatus) override;

Why doesn't nsICSSLoaderObserver's overload move to previous HTMLEditor() define?
Attachment #8979927 - Flags: review?(m_kato) → review+
Comment on attachment 8979928 [details]
Bug 1463327 - part 4: Add comments to explain which kind of methods should be public methods

https://reviewboard.mozilla.org/r/246122/#review252466
Attachment #8979928 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5c9278b7b027
part 1: Change scope of some methods of EditorBase which won't be called by non-helper classes of editing to protected r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f0cb4203b12d
part 2: Change scope of some methods of TextEditor which won't be called by non-helper classes of editing to protected r=m_kato
https://hg.mozilla.org/integration/autoland/rev/5d7484ed3b99
part 3: Change scope of some methods of HTMLEditor which won't be called by non-helper classes of editing to protected r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ac00527001f9
part 4: Add comments to explain which kind of methods should be public methods r=m_kato
You need to log in before you can comment on or make changes to this bug.