This is regression by bug 1543312.
By bug 1543312, we destroy editor when destroying PresShell. But when destroying PresShell, editor doesn't remove anonymous content of editing UI from PresShell. Then, when destroying the frame manager in `PresShell::Destroy`, it hits assertion due to uncomposed doc.
We shouldn't hide editing UI during destroying PresShell and we should hide it after destroyed.
Differential Revision: https://phabricator.services.mozilla.com/D42773
--HG--
extra : moz-landing-system : lando
It's called immediately before setting `mHTMLEditor` and sets `mHTMLEditor` to
`nullptr`. So, it does nothing actually. We can get rid of it.
Differential Revision: https://phabricator.services.mozilla.com/D42771
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::IsInlineNode()` is a wrapper of
`HTMLEditor::NodeIsInlineStatic()`, but returns opposite value.
This patch moves it into `HTMLEditor` and names it with same rule as
`NodeIsBlockStatic()`.
Note that this method may return true if given node is unexpected node type.
E.g., comment node, CDATA node, etc. However, it's not scope of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D42770
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::IsBlockNode()` just wraps `HTMLEditor::NodeIsBlockStatic()`
and all its users will be moved into `HTMLEditor`. Therefore, we should
unwrap it.
Differential Revision: https://phabricator.services.mozilla.com/D42769
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D42262
--HG--
extra : moz-landing-system : lando
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`.
Differential Revision: https://phabricator.services.mozilla.com/D42107
--HG--
extra : moz-landing-system : lando
The members of `HTMLEditRules` which are used only while `WillDoAction()` and
`DidDoAction()` are called should be moved to specific stack only struct
`EditorBase::EditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42106
--HG--
extra : moz-landing-system : lando
`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 `EditorRawDOMPoint`s, we don't need `mUtilRange`.
Differential Revision: https://phabricator.services.mozilla.com/D42105
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D42103
--HG--
extra : moz-landing-system : lando
For avoiding allocation cost of `RangeItem`, it should be stored by `HTMLEditor`
and reused by all `TopLevelEditSubActionData` instances for the editor instance.
Differential Revision: https://phabricator.services.mozilla.com/D42102
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D42096
--HG--
extra : moz-landing-system : lando
This condition was needed when FindInsertionPrevSibling and co didn't understand
display: contents.
Editor is pretty broken (and calls into PresShell::ContentRemoved directly, and
incorrectly, using anonymous nodes).
In this case we were taking the XBL path because of display: contents, which
means that we tried to seek to the editor anonymous node, and crash (since it's
not an explicit kid).
Editor needs to get fixed, but this is technically more correct and fixes the
crash, so we may as well take it in the interim.
Differential Revision: https://phabricator.services.mozilla.com/D42472
--HG--
extra : moz-landing-system : lando
They're bad, specially if they do vastly different thing in overloaded
functions, like aUseSystemPrincipal and aIsPreload. :)
Differential Revision: https://phabricator.services.mozilla.com/D40851
--HG--
extra : moz-landing-system : lando
`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`.
Differential Revision: https://phabricator.services.mozilla.com/D41401
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41400
--HG--
extra : moz-landing-system : lando
`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`.
Differential Revision: https://phabricator.services.mozilla.com/D41399
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41398
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41397
--HG--
extra : moz-landing-system : lando
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()`.
Differential Revision: https://phabricator.services.mozilla.com/D41396
--HG--
extra : moz-landing-system : lando
When browsing reported site (https://minecraft.curseforge.com/), user that uses Android cannot set caret on some editable contents.
GV's IME code uses `NOTIFY_IME_OF_SELECTION_CHANGE` to update selection on native IME. But GV doesn't receives this notification on this reported site. This notification is fired by editor's selection listener, but when this occurs, this notification is no logner fired unfortunately. Because selection listener isn't registered by current selection.
At first, editor registers selection listener by `EditorBase::Init`. But like test case, if PresShell is destroyed after editor is created, this registration is no longer used, and we have to register it by new selection again. So we should tear down editor when PresShell is destroyed. (or we have to add a way to re-initialize selection listener?)
This isn't related to Android. Android's IME code depends on this notification, so this occurs easily.
Also, `PresShell::SetCaretEnabled` only works when caret is valid (`PresShell::GetCaret` has caret). If caret is nothing, it hits assertion.
And, attached test case is reproduced sample.
Differential Revision: https://phabricator.services.mozilla.com/D41356
--HG--
extra : moz-landing-system : lando
`TextEditRules` and `HTMLEditRules` still refer
`EditorBase::mPaddingBRElementForEmptyEditor` directly but this is really ugly.
Therefore, this patch creates `EditorBase::HasPaddingBRElementForEmptyEditor()`
for wrapping its access.
Differential Revision: https://phabricator.services.mozilla.com/D41162
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41161
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::WillLoadHTML()` does exactly same thing as
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`. Therefore, we can
get rid of it and make `HTMLEditor::LoadHTML()` not use `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D41160
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41159
--HG--
extra : moz-landing-system : lando
`TextEditRules::RemoveRedundantTrailingBR()` is used only by multiline text
editor (i.e., `<textarea>`). Therefore, it should be moved into `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D41158
--HG--
extra : moz-landing-system : lando
`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
Differential Revision: https://phabricator.services.mozilla.com/D41157
--HG--
extra : moz-landing-system : lando
`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()`.
Differential Revision: https://phabricator.services.mozilla.com/D41156
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D41155
--HG--
extra : moz-landing-system : lando
This allows users of `WSRunScanner`'s functionality to pass a `const
HTMLEditor*`, allowing themselves to become const-correct.
Differential Revision: https://phabricator.services.mozilla.com/D41384
--HG--
extra : moz-landing-system : lando
We check surrogate pair at specific index in `nsTextFragement` in a lot of
places. This requires boundary check of the index so that it can cause
security issue and crash reason with simple mistake, and also it steals
your time to understand the code what it does especially when it's a
part of an `if` condition.
Therefore, this patch adds new API to `nsTextFragment` and makes the all
surrogate pair handlers of `nsTextFragument` use new API.
Differential Revision: https://phabricator.services.mozilla.com/D39689
--HG--
extra : moz-landing-system : lando
Original regression was by bug 1362858, and bug 1418629 wasn't enough to fix.
By bug 1362858, we use `CHAR_CLASS_SEPARATOR` in additional to DOM word separator. But some characters such as single quote, `@` and etc are `CHAR_CLASS_SEPARATOR`, so we may check spell by incomplete word.
We shouldn't separate word by characters that is email part, URL part or conditional punctuation.
And I also update test cases for this situation. `<textarea>` is better for spell checking since it can has multiple anonymous text nodes.
Differential Revision: https://phabricator.services.mozilla.com/D39829
--HG--
extra : moz-landing-system : lando
When using `<textarea>`, spell checker sometimes check spell when editing last text node. Because editor passes invalid previous selection node and offset via `HandleInlineSpellCheck`.
Although previous selected node uses `mCachedSelectionNode` that is stored by `BeforeEdit`, when this occurs, it isn't text node.
When editing last text node in `<textarea>`, anchror node might be root node, not current text node. So we should use text node instead when this is inserting text operation.
Differential Revision: https://phabricator.services.mozilla.com/D40368
--HG--
extra : moz-landing-system : lando
Stopping using attribute for "moz-br", `IMEContentObserver` cannot know when
new `<br>` element is changed to padding element for empty last line.
Therefore, editor needs to insert padding `<br>` element after setting the
flag properly. Then, `IMEContentObserver` does not need to recompute the
length of `<br>` element (if it's for padding, it computes the length as 0).
Unfortunately, `TextEditor::InsertBrElementWithTransaction()` is used in too
many places and it already has optional argument. Therefore, it's difficult
to change it. However, we should share the preparation before creating `<br>`
element in it with new method. Therefore, this patch creates
`EditorBase::PrepareToInsertBRElement()` to share the preparation point (almost
just moved from the method). Then, new method is created as
`EditorBase::InsertPaddingBRElementForEmptyLastLineWithTransaction()` because
it's used both in `TextEditor` and `HTMLEditor`. Finally, `TextEditor` won't
insert `<br>` element with `InsertBrElementWithTransaction()`. Therefore, it's
moved to `HTMLEditor` with renaming to `InsertBRElementWithTransaction()`.
Differential Revision: https://phabricator.services.mozilla.com/D39860
--HG--
extra : moz-landing-system : lando
`TextEditRules::CreateBR()` is used only by
`HTMLEditRules::InsertBRIfNeededInternal()` and it just calls
`TextEditor::InsertBrElementWithTransaction()`. Therefore, we can get rid of
it. Then, `CreateBRInternal()` can be renamed to
`CreatePaddingBRElementForEmptyLastLine()` since it's shared only by
`CreateBR()` and `CreatePaddingBRElementForEmptyLastLine()`.
Differential Revision: https://phabricator.services.mozilla.com/D39859
--HG--
extra : moz-landing-system : lando
