1572681 - Get rid of member variables of `TextEditRules`

Status: RESOLVED FIXED

(Core :: DOM: Editor, task)

Description

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

As far as I've investigated, TextEditRules needs only mTextEditor, mData and mIsHTMLEditRules. We can remove or move the other member variables.

Comment 1

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

Attachment: Bug 1572681 - part 1: Move `TextEditRules::mCachedSelection*` to `EditorBase::AutoEditActionDataSetter`

Despite of their names, TextEditRules::mCachedSelectionNode and
TextEditRules::mCachedSelectionOffset are used only for calling
EditorBase::HandleInlineSpellCheck() so that they should be renamed to
explain the purpose.

Additionally, they are not necessary to be in the heap since they are
necessary until TextEditRules::AfterEdit() is called. Therefore, we can
move them into EditorBase::HandleInlineSpellCheck().

Finally, TextEditRules::BeforeEdit() and TextEditRules::AfterEdit() are
called only by TextEditor::OnStartToHandleTopLevelEditSubAction() and
TextEditor::OnEndHandlingTopLevelEditSubAction(). Therefore, we can move
the setter to TextEditor::OnStartToHandleTopLevelEditSubAction().

Comment 2

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

Attachment: Bug 1572681 - part 2: Get rid of `TextEditRules::mTopLevelEditSubAction` and `TextEditRules::mActionNesting`

TextEditRules::BeforeEdit(), TextEditRules::AfterEdit(),
HTMLEditRules::BeforeEdit() and HTMLEditRules::AfterEdit() manages
TextEditRules::mActionNesting for preventing that they won't do same thing
per top-level edit sub-action. However, this has already been checked by
their caller, AutoTopLevelEditSubActionNotifier. So, we can get rid of it.
Then, TextEditRules::mTopLevelEditSubAction is also always same as
EditorBase::GetTopLevelEditSubAction(). Therefore, we can get rid of it
too.

Comment 3

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

Attachment: Bug 1572681 - part 3: Change `TextEditRules::mLockRulesSniffing` to `HTMLEditRules::mInitialized`

TextEditRules::mLockRulesSniffing is set by AutoLockRulesSniffing.
It's created during BeforeEdit() and AfterEdit() are called, and
HTMLEditRules::Init() is initializing mDocChangeRange.

The purpose of it is, preventing BeforeEdit() and AfterEdit() to do
something in that time. For the former cases, we don't need this member
anymore since they won't be nested. Therefore, we need to manage
HTMLEditRules::BeforeEdit() and HTMLEditRules::AfterEdit() won't do
anything only while HTMLEditRules::Init() is called. Therefore,
there should be only HTMLEditRules::mInitialized instead.

Comment 4

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

Attachment: Bug 1572681 - part 4: Get rid of arguments of `TextEditRules::BeforeEdit()` and `TextEditRules::AfterEdit()`

TextEditRules::BeforeEdit(), TextEditRules::AfterEdit(),
HTMLEditRules::BeforeEdit() and HTMLEditRules::AfterEdit() are always
called with same values as the result of
EditorBase::GetTopLevelEditSubAction() and
EditorBase::GetDirectionOfTopLevelEditSubAction().

For making what they do clearer, we should make them access with those
EditorBase members for now. This makes those methods ugly due to increasing
number of long lines. However, this issue should be solved when we move them
into TextEditor and HTMLEditor.

Comment 5

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

Attachment: Bug 1572681 - part 5: Get rid of `TextEditRules::mDeleteBidiImmediately`

TextEditRules::mDeleteBidiImmediately is cache of
bidi.edit.delete_immediately pref value at creation time of TextEditRules.
However, this is referred when user removes selection. So, there is no
reason to keep same behavior starting from editor creation. In other words,
it must be better to take same behavior in all editor instances.

Therefore, we should remove it and the pref value should be referred directly
when user tries to remove selection.

Comment 6

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

Attachment: Bug 1572681 - part 6: Move `TextEditRules::mDidExplicitlySetInterline` to `HTMLEditRules`

TextEditRules::mDidExplicitlySetInterline is set to true only by
HTMLEditRules, but TextEditRules::DidDeleteSelection() refers it.
So, it's enough to make TextEditRules::DidDeleteSelection() take the
value and we can move it into HTMLEditRules.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbd6bace05dc
part 1: Move `TextEditRules::mCachedSelection*` to `EditorBase::AutoEditActionDataSetter` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/3f5493ec4852
part 2: Get rid of `TextEditRules::mTopLevelEditSubAction` and `TextEditRules::mActionNesting` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f2bd71ff0ca3
part 3: Change `TextEditRules::mLockRulesSniffing` to `HTMLEditRules::mInitialized` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f83db3bd9f6f
part 4: Get rid of arguments of `TextEditRules::BeforeEdit()` and `TextEditRules::AfterEdit()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/1f06af74eb39
part 5: Get rid of `TextEditRules::mDeleteBidiImmediately` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a3c7baabe15e
part 6: Move `TextEditRules::mDidExplicitlySetInterline` to `HTMLEditRules` r=m_kato

Comment 8

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/cbd6bace05dc
https://hg.mozilla.org/mozilla-central/rev/3f5493ec4852
https://hg.mozilla.org/mozilla-central/rev/f2bd71ff0ca3
https://hg.mozilla.org/mozilla-central/rev/f83db3bd9f6f
https://hg.mozilla.org/mozilla-central/rev/1f06af74eb39
https://hg.mozilla.org/mozilla-central/rev/a3c7baabe15e