1572685 - Get rid of member variables of `HTMLEditRules`

Status: RESOLVED FIXED

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

Description

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

Most members in HTMLEditRules are cache required between BeforeEdit() and AfterEdit(). So, we can move them into EditorBase::AutoEditActionDataSetter.

Comment 1

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

Attachment: Bug 1572685 - part 1: Move `HTMLEditRules::mDidDeleteSelection` into `AutoEditActionDataSetter`

Most member variables of HTMLEditRules are temporary used while between
BeforeEdit() and AfterEdit() are called or WillDoAction() and
DidDoAction() are called.

The former means the data should be stored during top-level edit sub-action
is set. Therefore, this patch creates a struct, TopLevelEditSubActionData
and make it a member of AutoEditActionDataSetter. Then, makes
EditorBase::TopLevelEditActionDataRef() return reference of it.

And also this patch moves HTMLEditRules::mDidDeleteSelection into it.

Comment 2

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

Attachment: Bug 1572685 - part 2: Move `HTMLEditRules::mDidExplicitlySetInterLine` to `TopLevelEditSubActionData`

Comment 3

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

Attachment: Bug 1572685 - part 3: Move `HTMLEditRules::mDidRangedDelete` to `TopLevelEditSubActionData`

Comment 4

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

Attachment: Bug 1572685 - part 4: Move `HTMLEditRules::mDidEmptyParentBlocksRemoved` to `TopLevelEditSubActionData`

Comment 5

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

Attachment: Bug 1572685 - part 5: Move `HTMLEditRules::mRestoreContentEditableCount` to `TopLevelEditSubActionData`

Comment 6

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

Attachment: Bug 1572685 - part 6: Move `HTMLEditRules::mNewBlock` to `TopLevelEditSubActionData`

Comment 7

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

Attachment: Bug 1572685 - part 7: Move `HTMLEditRules::mRangeItem` to `TopLevelEditSubActionData`

For avoiding allocation cost of RangeItem, it should be stored by HTMLEditor
and reused by all TopLevelEditSubActionData instances for the editor instance.

Comment 8

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

Attachment: Bug 1572685 - part 8: Move `HTMLEditRules::mCachedStyles` to `TopLevelEditSubAction`

This patch makes StyleCache not inherit ItemProp because MOZ_COUNT_CTOR
and MOZ_COUNT_DTOR do not work well with AutoTArray and there is no reason
to do that since nobody treat StyleCache instance with ItemProp pointer.

Comment 9

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

Attachment: Bug 1572685 - part 9: Move `HTMLEditRules::mDocChangeRange` to `TopLevelEditSubActionData`

Comment 10

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

Attachment: Bug 1572685 - part 10: Get rid of `HTMLEditRules::mUtilRange`

HTMLEditRules::mUtilRange is used only for the argument of
HTMLEditRules::UpdateDocChangeRange(). However, it does not need to be
a range instance since it compares its start and
TopLevelEditSubAction::mChangedRange->StartRef(), and its end and
TopLevelEditSubAction::mChangedRange->EndRef(). Therefore, with rewriting
it as taking 2 EditorRawDOMPoints, we don't need mUtilRange.

Comment 11

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

Attachment: Bug 1572685 - part 11: Move `HTMLEditRules::JoinOffset` to `EditorBase::EditSubActionData`

The members of HTMLEditRules which are used only while WillDoAction() and
DidDoAction() are called should be moved to specific stack only struct
EditorBase::EditSubActionData.

Comment 12

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

Attachment: Bug 1572685 - part 12: Move `HTMLEditRules::mListenerEnabled` to `EditSubActionData`

This patch includes a logic change. HTMLEditRules::mListenerEnabled is used
for storing 2 state. One is that whether HTMLEditRules can handle the legacy
listener methods (they came from nsIEditActionListener) with mHTMLEditor or
not. The other is that whether current edit sub-action handler temporarily
disables the listeners for performance or not.

For the former, we can check same result with HTMLEditRules::mInitialized
and HTMLEditRules::mHTMLEditor.

However, the latter, current design is obviously wrong. Currently,
HTMLEditRules::mListenerEnabled is set only to false temporarily from
WillInsertText(), but the state will be affected to other edit sub-actions
which are nested by mutation event listeners. So, currently,
HTMLEditRules::mDocChangeRange may not contain modified range caused by
nested edit sub-actions.

For solving this bug, this patch moves it into EditSubActionData rather
than HTMLEditor and TopLevelEditSubActionData.

Comment 13

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

Attachment: Bug 1572685 - part 13: Get rid of `HTMLEditRules::mReturnInEmptyLIKillsList`

HTMLEditRules::mReturnInEmptyLIKillsList stores value of
editor.html.typing.returnInEmptyListItemClosesList at construction, and it's
set to true unless the pref value is "false". However, this pref isn't
registered in anywhere (all.js, firefox.js, etc) nor used in comm-central
and BlueGriffon.

Even if I search the pref in the web, I don't see any information so that
this hidden pref must not be used anybody. Therefore, this patch just removes
this member.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4707a5e44d23
part 1: Move `HTMLEditRules::mDidDeleteSelection` into `AutoEditActionDataSetter` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/de4d785d8268
part 2: Move `HTMLEditRules::mDidExplicitlySetInterLine` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/35e9843249ad
part 3: Move `HTMLEditRules::mDidRangedDelete` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fb3f0029026a
part 4: Move `HTMLEditRules::mDidEmptyParentBlocksRemoved` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9db84872dfb4
part 5: Move `HTMLEditRules::mRestoreContentEditableCount` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/bb657e30fe2a
part 6: Move `HTMLEditRules::mNewBlock` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/72900ea22020
part 7: Move `HTMLEditRules::mRangeItem` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a7453a2a1713
part 8: Move `HTMLEditRules::mCachedStyles` to `TopLevelEditSubAction` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e07b7a1a595b
part 9: Move `HTMLEditRules::mDocChangeRange` to `TopLevelEditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/193653b7d3f2
part 10: Get rid of `HTMLEditRules::mUtilRange` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6de55c5b5f8d
part 11: Move `HTMLEditRules::JoinOffset` to `EditorBase::EditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/7ac5164efa3e
part 12: Move `HTMLEditRules::mListenerEnabled` to `EditSubActionData` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/df563a5f853a
part 13: Get rid of `HTMLEditRules::mReturnInEmptyLIKillsList` r=m_kato

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/4707a5e44d23
https://hg.mozilla.org/mozilla-central/rev/de4d785d8268
https://hg.mozilla.org/mozilla-central/rev/35e9843249ad
https://hg.mozilla.org/mozilla-central/rev/fb3f0029026a
https://hg.mozilla.org/mozilla-central/rev/9db84872dfb4
https://hg.mozilla.org/mozilla-central/rev/bb657e30fe2a
https://hg.mozilla.org/mozilla-central/rev/72900ea22020
https://hg.mozilla.org/mozilla-central/rev/a7453a2a1713
https://hg.mozilla.org/mozilla-central/rev/e07b7a1a595b
https://hg.mozilla.org/mozilla-central/rev/193653b7d3f2
https://hg.mozilla.org/mozilla-central/rev/6de55c5b5f8d
https://hg.mozilla.org/mozilla-central/rev/7ac5164efa3e
https://hg.mozilla.org/mozilla-central/rev/df563a5f853a