Move `TextEditRules::mPaddingBRElementForEmptyEditor` to `EditorBase`
Categories
(Core :: DOM: Editor, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(8 files)
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review |
We should move TextEditRules::mPaddingBRElementForEmptyEditor to EditorBase for first step of moving all members in TextEditRules and HTMLEditRules to editor classes.
|
Assignee | |
Comment 1•5 years ago
|
||
TextEditRules::mPaddingBRElementForEmptyEditor are used by both TextEditor
and HTMLEditor. Therefore, it should be in EditorBase.
This patch makes TextEditRules and HTMLEditRules directly access the
private member of EditorBase temporarily. It'll be fixed by the following
patches.
|
|
Assignee | |
Comment 2•5 years ago
|
||
TextEditRules::WillInsert() is not used with initial purpose since
HTMLEditor always works with HTMLEditRules and its WillDoAction()
always handles EditSubAction::eInsertElement.
Additionally, its name is too generic since it does non-related 3 things.
One is checking whether the editor is readonly or disabled. However, this
may not be necessary since its callers may have already checked it or
just ignored the result. So, this should be check by each caller.
Next one is masking password if auto-masking is enabled. This is TextEditor
specific feature so that this patch moves the code into
TextEditor::MaybeDoAutoPasswordMasking().
Final one is removing empty <br> element for empty editor if there is.
This is common feature so that this patch moves this code into
EditorBase::EnsureNoPaddingBRElementForEmptyEditor().
|
|
Assignee | |
Comment 3•5 years ago
|
||
TextEditRules::WillUndo() and TextEditRules::WillRedo() only check whether
the editor is readonly/disabled or not. So, TextEditor::UndoAsAction() and
TextEditor::RedoAsAction() should do it first.
TextEditRules::DidUndo() and TextEditRules::DidRedo() only set or unset
mPaddingBRElementForEmptyEditor if it's restored by undo or redo.
Therefore, we can move the code into TextEditor::UndoAsAction() and
TextEditor::RedoAsAction().
Note that this patch makes TextEditor::UndoAsAction() discard the result of
TransactionManager::Undo() because this is inconsistent from what
TextEditor::RedoAsAction() does and this was changed by part 5 of bug 1447924.
https://hg.mozilla.org/mozilla-central/rev/869a1445816be7f43f54f7c97f28e4c6273fa75f
|
|
Assignee | |
Comment 4•5 years ago
|
||
TextEditRules::RemoveRedundantTrailingBR() is used only by multiline text
editor (i.e., <textarea>). Therefore, it should be moved into TextEditor.
|
|
Assignee | |
Comment 5•5 years ago
|
||
TextEditRules::CreatePaddingBRElementForEmptyEditorIfNeeded() is used by
both TextEditor and HTMLEditor so that this moves it into EditorBase.
Additionally, making TextEditor::MaybeChangePaddingBRElementForEmptyEditor()
call it if there is no content. Then, we can avoid the dependency of them.
|
|
Assignee | |
Comment 6•5 years ago
|
||
HTMLEditRules::WillLoadHTML() does exactly same thing as
EditorBase::EnsureNoPaddingBRElementForEmptyEditor(). Therefore, we can
get rid of it and make HTMLEditor::LoadHTML() not use HTMLEditRules.
|
|
Assignee | |
Comment 7•5 years ago
|
||
HTMLEditRules::OnModifyDocument() is same as just calling
EditorBase::EnsureNoPaddingBRElementForEmptyEditor() and
EditorBase::MaybeCreatePaddingBRElementForEmptyEditor() so that this patch
gets rid of the method, then, creates HTMLEditor::OnModifyDocumentInternal()
and makes it do same thing.
|
|
Assignee | |
Comment 8•5 years ago
|
||
TextEditRules and HTMLEditRules still refer
EditorBase::mPaddingBRElementForEmptyEditor directly but this is really ugly.
Therefore, this patch creates EditorBase::HasPaddingBRElementForEmptyEditor()
for wrapping its access.
|
||
Updated•5 years ago
|
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/235c10b6af84 part 1: Move `TextEditRules::mPaddingBRElementForEmptyEditor` to `EditorBase` r=m_kato https://hg.mozilla.org/integration/autoland/rev/94b4e63c71df part 2: Split `TextEditRules::WillInsert()` r=m_kato https://hg.mozilla.org/integration/autoland/rev/e7d953d0be6c part 3: Get rid of `TextEditRules::WillUndo()`, `TextEditRules::DidUndo()`, `TextEditRules::WillRedo()` and `TextEditRules::DidRedo()` r=m_kato https://hg.mozilla.org/integration/autoland/rev/8059efaac8ca part 4: Move `TextEditRules::RemoveRedundantTrailingBR()` into `TextEditor` r=m_kato https://hg.mozilla.org/integration/autoland/rev/8c7339d7cd3b part 5: Move `TextEditRules::CreatePaddingBRElementForEmptyEditorIfNeeded()` into `EditorBase` r=m_kato https://hg.mozilla.org/integration/autoland/rev/e14e02842d3e part 6: Get rid of `HTMLEditRules::WillLoadHTML()` r=m_kato https://hg.mozilla.org/integration/autoland/rev/571000b4df5b part 7: Get rid of `HTMLEditRules::OnModifyDocument()` r=m_kato https://hg.mozilla.org/integration/autoland/rev/3a07d2f4b955 part 8: Wrap `EditorBase::mPaddingBRElementForEmptyEditor` with an inline method r=m_kato
|
||
Comment 10•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/235c10b6af84
https://hg.mozilla.org/mozilla-central/rev/94b4e63c71df
https://hg.mozilla.org/mozilla-central/rev/e7d953d0be6c
https://hg.mozilla.org/mozilla-central/rev/8059efaac8ca
https://hg.mozilla.org/mozilla-central/rev/8c7339d7cd3b
https://hg.mozilla.org/mozilla-central/rev/e14e02842d3e
https://hg.mozilla.org/mozilla-central/rev/571000b4df5b
https://hg.mozilla.org/mozilla-central/rev/3a07d2f4b955
Description
•