1717156 - Get rid of some unused `nsIEditor` flags

Status: RESOLVED FIXED

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

Comment 1

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

Attachment: Bug 1717156 - part 1: Make editor type specific flags clearer with `MOZ_ASSERT` r=m_kato!

Some nsIEditor::eEditor*Mask flags are now only for TextEditor or
HTMLEditor. For making it clearer, add MOZ_ASSERT to the SetFlags and
each flag accessor.

Depends on D118246

Comment 2

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

Attachment: Bug 1717156 - part 2: Get rid of `nsIEditor::eEditorWidgetMask` and its accessor r=m_kato!

With the previous patch, we know nsIEditor::eEditorWidgetMask always
matches with EditorBase::IsTextEditor(). And it's not referred from JS
(including comm-central and BlueGriffon). So, we can get rid of it.

Depends on D118261

Comment 3

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

Attachment: Bug 1717156 - part 3: Get rid of `nsIEditor::eEditorFilterInputMask` and its accessor r=m_kato!

Nobody (including comm-central and BlueGriffon) uses
nsIEditor::eEditorFilterInputMask. Therefore, we can get rid of this and its
accessor, EditorBase::IsInputFiltered().

Depends on D118262

Comment 4

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

Attachment: Bug 1717156 - part 4: Drop `Tab` key handling in `EditorBase` r=m_kato!

Now, EditorBase::IsTabable() returns true only when it's an HTMLEditor
instance and nsIEditor::eEditorAllowInteraction is not set. Additionally,
nobody controls the flag of TextEditor. So, we can make the flag available
only with HTMLEditor instance, and Tab key handling in EditorBase
can be moved to HTMLEditor.

Depends on D118263

Comment 5

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

Attachment: Bug 1717156 - part 5: Get rid of `nsIEditor::eEditorNoCSSMask` r=m_kato!

Now, nsIEditor::eEditorNoCSSMask is used only in the editor internally.
And it's available and meaningful only with HTMLEditor instance. So,
we can get rid of it from EditorBase. Fortunately, HTMLEditor always
creates CSSEditUtils and it stores the raw value indicating whether the
editor manager enabled or disabled CSS. Therefore, we don't need new
member variable in HTMLEditor for storing the flag. And this allows us
to remove nsIEditor::SetFlags() override of HTMLEditor.

Depends on D118264

Comment 6

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

Attachment: Bug 1717156 - part 6: Get rid of `nsIEditor::eEditorDontEchoPassword` r=m_kato!

It's used only by password field, i.e., only by TextEditor, and used
temporarily. Additionally, there is some space in TextEditor. So, we
can get rid of it, and make TextEditor store it directly.

Note that this allows to skip expensive nsIEditor::SetFlags() calls by
AutoRestoreEditorState. This may improve setting <input>.value performance.

Depends on D118265

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b0d8e650ee5
part 1: Make editor type specific flags clearer with `MOZ_ASSERT` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/b7cca2c3c59f
part 2: Get rid of `nsIEditor::eEditorWidgetMask` and its accessor r=m_kato
https://hg.mozilla.org/integration/autoland/rev/43ef6c46da25
part 3: Get rid of `nsIEditor::eEditorFilterInputMask` and its accessor r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a2499b7e29ca
part 4: Drop `Tab` key handling in `EditorBase` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/5f8d37c41d72
part 5: Get rid of `nsIEditor::eEditorNoCSSMask` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/b6d0dc2e5c7d
part 6: Get rid of `nsIEditor::eEditorDontEchoPassword` r=m_kato

Comment 8

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

Attachment: Bug 1717156 - part 7: Revert the change of the debug `printf` in `TextEditor.cpp` r=m_kato!

I forgot to revert this before landing.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/715eceba0790
part 7: Revert the change of the debug `printf` in `TextEditor.cpp` r=m_kato

Comment 10

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/3b0d8e650ee5
https://hg.mozilla.org/mozilla-central/rev/b7cca2c3c59f
https://hg.mozilla.org/mozilla-central/rev/43ef6c46da25
https://hg.mozilla.org/mozilla-central/rev/a2499b7e29ca
https://hg.mozilla.org/mozilla-central/rev/5f8d37c41d72
https://hg.mozilla.org/mozilla-central/rev/b6d0dc2e5c7d
https://hg.mozilla.org/mozilla-central/rev/715eceba0790

Comment 11

[Geoff Lankow (:darktrojan)]

Thunderbird is asserting here every time we open a compose window. Do we need to do something to match your changes, or is something not quite right?

Comment 12

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

(In reply to Geoff Lankow (:darktrojan) from comment #11)

Thunderbird is asserting here every time we open a compose window. Do we need to do something to match your changes, or is something not quite right?

According to the symptom, it seems that the crash occurs here:
https://searchfox.org/comm-central/rev/bc3bbc6f9a332c72ffe9d53c28cdd055af51d8fb/mail/components/compose/content/MsgComposeCommands.js#9111

I missed to catch this line. So, dropping MOZ_ASSERTs in Init and IsMailEditor is fine. Although the flag usage is really hacky (TextEditor itself does not change the behavior, but the flag is editor's own).