In theory, methods which touch the DOM tree should take `EditorDOMPoint` rather
than `EditorRawDOMPoint`. And now, we can return meaningful value if it
succeeded, with `Result`.
Therefore, this patch makes it return `Result<EditorDOMPoint, nsresult>` instead
of taking an out argument, and take only `EditorDOMPoint` rather than taking any
type of `EditorDOMPointBase` since it's always converted to `EditorDOMPoint`.
Differential Revision: https://phabricator.services.mozilla.com/D143881
To avoid the unnecessary conversion between `EditorDOMPoint` and
`EditorRawDOMPoint`, it should be a template method which can take/return either
`EditorDOMPoint` or `EditorRawDOMPoint`.
Differential Revision: https://phabricator.services.mozilla.com/D143880
The implicit copy constructors and `operator=` makes it harder to realize
that implicit conversion wastes the runtime cost. Therefore, this patch
replaces them with a template method to convert the `EditorDOMPointBase` type.
Differential Revision: https://phabricator.services.mozilla.com/D143878
First, move methods of `HTMLEditor` which collapse `Selection` to `EditorBase`.
Then, make editor stop accessing `Selection::CollapseInLimiter` directly.
Differential Revision: https://phabricator.services.mozilla.com/D143814
It's hard to remember what the `bool` value of them means. Therefore, there
are a lot of comment around the API users to explain what they do. For making
it clearer and reducing the risk of specifying opposite value, they should work
with an `enum class`.
Differential Revision: https://phabricator.services.mozilla.com/D143812
Due to breaking `mobile.twitter.com/explore` for Hebrew IME users on Android,
this patch backs out the patches for bug 1753420 except the last patch which
just rename some methods and variables.
Differential Revision: https://phabricator.services.mozilla.com/D143684
They return IME selection range rather than composition string range. I.e.,
when the composition does not have IME selections anymore, e.g., at committing,
they return non-set `RawRangeBoundary`.
For making the result meaning clearer, they should be
`FirstIMESelectionStartRef` and `LastIMESelectionEndRef`.
Depends on D141194
Differential Revision: https://phabricator.services.mozilla.com/D141195
Currently, it's done at creating `CompositionTransaction`, not after executing
it. Let's clean it up now.
Depends on D141193
Differential Revision: https://phabricator.services.mozilla.com/D141194
When `Selection` instance is updated, the old selection may be in batch.
In the case, `UpdateSelectionCache` should clean up the batch in the old
selection and start new one in the new selection instead.
Differential Revision: https://phabricator.services.mozilla.com/D139349
For consistency with the other things in editor module, it should be "nodes"
rather than "node" since 2 nodes are joined by it.
Differential Revision: https://phabricator.services.mozilla.com/D139715
Perhaps due to a bug Selection and/or Range API, selection may be in a native
anonymous subtree, and if the content is editable like in anonymous `<div>`
element in `<input>` or `<textarea>`, `HTMLEditor` may put unexpected element
into the anonymous `<div>` element.
For avoiding it, `EditorBase::IsSelectionEditable()` should return `false`
when it detects this odd situation. Then, editing commands do not work in
the anonymous subtree.
Differential Revision: https://phabricator.services.mozilla.com/D134658
Offset in a node is declared as "unsigned long" by the standards and we don't
limit node can have less than `INT32_MAX`. So it should return `uint32_t`, but
it also needs to represent the case of "not found". For consistency with some
other APIs like `nsContentUtils::ComparePoints`, using `Maybe` must be a good
style rather than `Result<uint32_t, bool>`.
This patch fixes the callers in assertions for example.
Differential Revision: https://phabricator.services.mozilla.com/D131335
It's hard to fix some callers. Therefore, in this bug, we should fix only
simple cases. Therefore, we should rename existing API first.
Differential Revision: https://phabricator.services.mozilla.com/D131334
It's an internal API corresponding to `Selection.getRangeAt` DOM API.
I think that it should use `uint32_t` rather than `size_t` because of the
consistency with the DOM API and `Selection::RangeCount()`.
This patch fixes all callers of `GetRangeAt()`, and rewrites it with ranged-
loops unless original ones do not refer `RangeCount()` every time and may run
script in the loop.
Differential Revision: https://phabricator.services.mozilla.com/D128848
Our internal code can touch internal `Selection` object of `<input>` and
`<textarea>`, thus, selection ranges in them may be not in the text node.
Therefore, we should make `TextEditor` instance try to shrink the deleting
ranges into the text node first.
In a followup bug (perhaps, bug 1734846), we should split the deletion code into
`TextEditor` and `HTMLEditor` for making the code simpler and never regressed.
Differential Revision: https://phabricator.services.mozilla.com/D133181
I realized that it's now used only by `HTMLEditor` so that we can move it
into `HTMLEditor` and we can make `CreateElementTransaction` treat `HTMLEditor`
directly rather than via `EditorBase`.
Differential Revision: https://phabricator.services.mozilla.com/D131198
Currently, it assumes that new node is the left node and split node is the
right node. However, we need to make it possible to handle the case that
right node is new one for fixing bug 1735608.
Depends on D130458
Differential Revision: https://phabricator.services.mozilla.com/D130624
`TopLevelEditSubActionData::WillJoinNodes()` and
`TopLevelEditSubActionData::DidJoinNodes()` are called only by
`HTMLEditor::JoinNodesWithTransaction()`. `WillJoinNodes()` assumes that
all children or text data in `aLeftContent` is moved to head of `aRightContent`.
Therefore, it just stores length of `aLeftContent` and `DidJoinNodes()` lets
`AddPointToChangedRange()` know the joined point.
Same value is stored by `HTMLEditor::JoinNodesWithTransaction()`. Therefore,
it can create same DOM point at calling `DidJoinNodes()` so that we can get
rid of `WillJoinNodes()`.
Differential Revision: https://phabricator.services.mozilla.com/D130348
`TopLevelEditSubActionData::WillJoinNodes()` and
`TopLevelEditSubActionData::DidJoinNodes()` are called only by
`HTMLEditor::JoinNodesWithTransaction()`. `WillJoinNodes()` assumes that
all children or text data in `aLeftContent` is moved to head of `aRightContent`.
Therefore, it just stores length of `aLeftContent` and `DidJoinNodes()` lets
`AddPointToChangedRange()` know the joined point.
Same value is stored by `HTMLEditor::JoinNodesWithTransaction()`. Therefore,
it can create same DOM point at calling `DidJoinNodes()` so that we can get
rid of `WillJoinNodes()`.
Differential Revision: https://phabricator.services.mozilla.com/D130348
Currently, `EditorBase::EndUpdateViewBatch()` does this, but it's before `input`
event. Therefore, if an `input` event listener changes the target elements'
position and/or size, user will see broken UI. Therefore, it should be updated
at editor finishes everything.
Differential Revision: https://phabricator.services.mozilla.com/D128871
`nsFocusManager` does not send `focus` event in some cases, e.g., non-editable
root element gets focus. However, the element may become editable later without
a focus move. Therefore, even if `IMEStateManager::sContent` is `nullptr`,
`IMEStateManager::UpdateIMEState()` and `IMEStateManager::FocusInEditor()` are
called with focused content when the uncomposed document is in design mode.
With this change, editor does not need to call `IMEStateManager` methods with
`nullptr` when it's in `designMode`. Therefore, we can get rid of
`GetFocusedContentForIME()` which just returns `nullptr` if focused content is
in design mode.
Differential Revision: https://phabricator.services.mozilla.com/D127612
There are a lot of check of `Document`'s editable state **with** comments. This
means that it's unclear for developers that only `Document` node is editable in
design mode.
Additionally, there are some points which use composed document rather than
uncomposed document even though the raw API uses uncomposed document. Comparing
with the other browsers, checking uncomposed document is compatible behavior,
i.e., nodes in shadow trees are not editable unless `contenteditable`.
Therefore, `nsINode` should have a method to check whether it's in design mode
or not.
Note that it may be called with a node in UA widget. Therefore, this patch
adds new checks if it's in UA widget subtree or native anonymous subtree,
checking whether it's in design mode with its host.
Differential Revision: https://phabricator.services.mozilla.com/D126764
Although we don't have a testcase, `HTMLEditor` may need to delete a text node
if it's created for composition. If it's not allowed, users may see staying
composition string in disabled editor.
Differential Revision: https://phabricator.services.mozilla.com/D125502
Similar to the previous patch, this patch makes `insertLineBreak` command
handler of `HTMLEditor` may insert a linefeed character instead of `<br>`
element for compatibility with Blink/WebKit.
Note that for making same name rules as `insertParagraphSeparator` command
handlers, this patch renames
`HTMLEditor::InsertBRElementAtSelectionWithTransaction()` to
`InsertLineBreakAsSubAction()` and moves
`EditorBase::InsertLineBreakAsSubAction()` to `TextEditor` since it's
used only by `TextEditor` instance.
Depends on D124731
Differential Revision: https://phabricator.services.mozilla.com/D124732
I just missed some callers of it. And I also make
`EditorBase::DeleteNodeTransaction()` a virtual method and `HTMLEditor` override
it. Then, even if it's called by helper classes, the `HTMLEditor` version is
always called.
Differential Revision: https://phabricator.services.mozilla.com/D123578
Web apps can modify normal selection even during IME composition and no
browsers stop composition by it. However, our editor tries to delete
non-collapsed selected range before updating composition. Therefore,
we need additional state at handling inserting text whether selection
should be deleted or ignored.
Depends on D121371
Differential Revision: https://phabricator.services.mozilla.com/D121372
Unfortunately, marking its constructor and destructor as `MOZ_CAN_RUN_SCRIPT`,
`Maybe<AutoSelectionRestorer>::reset()` and
`Maybe<AutoSelectionRestorer>::emplace()` cause bustage. Therefore, this patch
just mark them as `MOZ_CAN_RUN_SCRIPT_BOUNDARY`.
Note that `EditorBase::SavedSelectionRef()` cannot be moved to `HTMLEditor`
because `mEditActionData` is a private member of `EditorBase`.
Depends on D119001
Differential Revision: https://phabricator.services.mozilla.com/D119002
It should be treated as `uint32_t` since DOM API does so. However, there are
some exceptions:
* Result of `nsINode::ComputeIndexOf()`
* Result of `nsAString` methods
They return `-1` as not found, and anyway, they cannot treat large integer
than `INT32_MAX`. Therefore, this patch does not touch around them.
Differential Revision: https://phabricator.services.mozilla.com/D118933
During a `TextControlState` alive, `PasswordMaskData` should be alive too.
Otherwise, we cannot keep unmasked range at reframing.
Depends on D118756
Differential Revision: https://phabricator.services.mozilla.com/D118757
In my understanding at fixing bug 1717156, `nsIEditor.eEditorMailMask` won't be
set to `TextEditor` instance. However for making consistent **spellchecker**
behavior on email composer, subject editor is also set this flag. So, we need
to drop the check in `SetFlags` and IsMailEditor.
Differential Revision: https://phabricator.services.mozilla.com/D118561
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.
Differential Revision: https://phabricator.services.mozilla.com/D118266
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`.
Differential Revision: https://phabricator.services.mozilla.com/D118265
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`.
Differential Revision: https://phabricator.services.mozilla.com/D118264
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.
Differential Revision: https://phabricator.services.mozilla.com/D118262
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.
Differential Revision: https://phabricator.services.mozilla.com/D118261
Developers may be confused at `IsTextEditor()` and `IsPlaintextEditor()`. When
the latter is `true`, the former is always `true`, but it may be `true` when the
editor is `HTMLEditor` too. So, it's a mode of `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D118246
It's a static method and called only with `EditorDOMPointBase` instances.
So, we can add new API to `EditorDOMPointBase` instead.
Note that it's changed to return container when the container is a data node
rather than a text node. This should be better because no data node can
have children.
Depends on D117382
Differential Revision: https://phabricator.services.mozilla.com/D117490
For consistency with the similar internal DOM API, `As*()` should just cast
the type without checking editor type. Instead, `GetAs*()` should do it.
Differential Revision: https://phabricator.services.mozilla.com/D117381
It should be a virtual method derived from `EditorBase`, and `TextEditor`
and `HTMLEditor` should override it. Then, `nsIEditor::Paste()` requires
referring vtable again if we keep implementing it only in `EditorBase`.
Therefore, this patch avoid it with implementing it in both `TextEditor`
and `HTMLEditor`.
Depends on D116567
Differential Revision: https://phabricator.services.mozilla.com/D116568
It just creates an `nsITransferable` instance and add 2 flavors for storing
plain text. Therefore, it can be in `EditorUtils` instead.
Depends on D116566
Differential Revision: https://phabricator.services.mozilla.com/D116567
It stopped using `ComputeValueFromTextNodeAndBRElement` for `HTMLEditor` case.
However, `ComputeValueFromTextNodeAndBRElement` handles the case that there is
only padding `<br>` element for empty editor even if the instance is
`HTMLEditor`.
So, this patch makes it handle this special case by itself before checking
whether the instance is `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D116541
`Delete` and `Backspace` keys are handled by same code. So, the code should
be in `EditorBase` instead of `TextEditor`.
If `HTMLEditor` is in the plaintext editing mode of mail composer, `Tab` key
is also handled by the same code as `TextEditor`. So, the code in `TextEditor`
should be moved to `EditorBase` too and `HTMLEditor` should call `EditorBase`'s
method only when it's in the plaintext mode.
Depends on D116352
Differential Revision: https://phabricator.services.mozilla.com/D116353
Such events shouldn't be fired, but for now, we should make them handled in
the overrides. Then, we can avoid the skipping `TextEditor` case from
`HTMLEditor`.
Depends on D116351
Differential Revision: https://phabricator.services.mozilla.com/D116352
In any types of editor, `EditorBase` handles if it's in the read-only mode.
For making `HandleKeyPressEvent` relation between classes simpler, let's
handle it in an independent method.
Depends on D116350
Differential Revision: https://phabricator.services.mozilla.com/D116351
It's used both with `TextEditor` instance and `HTMLEditor` instance. So, it
should be implemented in `EditorBase`.
Depends on D116349
Differential Revision: https://phabricator.services.mozilla.com/D116350
It's a helper method of `TextEditor::ComputeValueInternal()` which is used by
`TextEditor` and `HTMLEditor::Rewrap()`. So, before we move
`ComputeValueInternal()`, we need to move this first.
Depends on D116347
Differential Revision: https://phabricator.services.mozilla.com/D116348
The attribute is used only with `HTMLEditor`, and it does not make sense to
access document's character-set via `TextEditor`. Therefore, this patch
makes it implement in `HTMLEditor` (`EditorBase` will return
`NS_ERROR_NOT_AVAILABLE` both getter and setter).
Note that `EditorBase::GetDocumentCharsetInternal()` is required by
`TextEditor::ComputeValueInternal()`. Therefore, it needs to stay in
`EditorBase`.
Differential Revision: https://phabricator.services.mozilla.com/D115948
`TextEditor` declares some virtual methods newly. However, for moving some
methods from `TextEditor` to `EditorBase`, they should be accessible from
`EditorBase`. Therefore, this patch adds declarations of pure virtual
methods of them to `EditorBase`.
Differential Revision: https://phabricator.services.mozilla.com/D115796
It's common method of `TextEditor` and `HTMLEditor`, but implemented by
`TextEditor` even though it's an override of `nsIEditor`'s method.
Therefore, it should be implemented in `EditorBase` instead.
Differential Revision: https://phabricator.services.mozilla.com/D115794
Of course, they are used for `HTMLEditor` instances too. Therefore, they
should be in `EditorBase` rather than `TextEditor`.
Depends on D115789
Differential Revision: https://phabricator.services.mozilla.com/D115790
They just work with a transaction manager and transactions, and they are used
by both `TextEditor` and `HTMLEditor`. Therefore, they should be in
`EditorBase` for making `HTMLEditor` stop inheriting `TextEditor` in the
future.
Depends on D115787
Differential Revision: https://phabricator.services.mozilla.com/D115788
Currently, `EditorBase::GetDocumentIsEmpty()` is implemented by `TextEditor`,
and it refers only `IsEmpty()` which is implemented both by `TextEditor` and
`HTMLEditor`. So, `IsEmpty()` should be a virtual method of `EditorBase`,
then, `EditorBase` can implement `GetDocumentIsEmpty()`.
Depends on D115786
Differential Revision: https://phabricator.services.mozilla.com/D115787
They are used by setting text value of `TextEditor` or replacing a misspelled
word with a new word in both `TextEditor` and `HTMLEditor`. Therefore,
they should be in the `EditorBase` rather than `TextEditor`.
Note that the path of the former case may be in a hot path. Therefore, we need
to keep redirecting to `TextEditor` for keeping the performance only in the
case.
Depends on D115785
Differential Revision: https://phabricator.services.mozilla.com/D115786
This method is semi-public method, meaning that this is commonly used by
public methods which handle various user input and that causes inputting
text, both in `TextEditor` and `HTMLEditor`.
Therefore, for making `HTMLEditor` stop inheriting `TextEditor` class in the
future, we should move it into `EditorBase`.
Depends on D115784
Differential Revision: https://phabricator.services.mozilla.com/D115785
IME is available in both `TextEditor` and `HTMLEditor`, and the handling
code is almost same (they partially do different things with checking
`IsHTMLEditor()`). Therefore, we should move them to `EditorBase` for
making `HTMLEditor` possible to inherit only `EditorBase` in the future.
Differential Revision: https://phabricator.services.mozilla.com/D115784
And also `HTMLEditor::CountEditableChildren()` is moved since it's used only by
it.
Despite the long method name, it's really unclear what it does. I try to
explain it with new name, but the things which the method does are not make
sense. So, if you have better name idea, I'll take it...
Differential Revision: https://phabricator.services.mozilla.com/D115177
I realized that it's used only by the dead path because the only caller,
`EditorBase::BeginningOfDocument()` is overridden by `HTMLEditor` and
is never called.
Differential Revision: https://phabricator.services.mozilla.com/D115117
If the editor instance is a `TextEditor`, the root element has to be
anonymous `<div>` element and it has only one text node when it has non-empty
value. Therefore, if it's in `TextEditor`, the method does not need to use
the complicated APIs for finding a text node from the anonymous `<div>` element
or padding `<br>` element since it can adjust the given point into the text
node without such API.
Differential Revision: https://phabricator.services.mozilla.com/D113240
Only the user is `EditorBase::BeginningOfDocument()` which is used by both
`TextEditor` and `HTMLEditor`. However, if it's a `TextEditor`, expected
result is only the first text node only when there is one. Therefore, we can
move it into `HTMLEditUtils` and make `EditorBase::BeginningOfDocument()`
find the text node by it self. Then, we can get rid of using
`EditorBase::GetEditorType()` in this case.
Differential Revision: https://phabricator.services.mozilla.com/D113239
They are instance members of `EditorBase` only for referring editor type and
editing host. Therefore, we can make them static with making them take
editor type and ancestor limiter as arguments.
Depends on D113236
Differential Revision: https://phabricator.services.mozilla.com/D113237
The following patches will get rid of inline wrapper methods of them.
Therefore, they shouldn't be called as "internal". And they return
`nsIContent*` instead of `nsINode*` so, `Get*Content()` is better name for
them.
Depends on D113229
Differential Revision: https://phabricator.services.mozilla.com/D113230
So, the meaning is reverted of this action. But with this change, the scanner
methods scans any nodes by default. This is simpler to understand from the
callers.
Depends on D113228
Differential Revision: https://phabricator.services.mozilla.com/D113229
Before moving them from `EditorBase`, they should take only one option argument
whose type is an `EnumSet` class.
Note that I wanted to name each option for optional behavior, but
`FindAnyDataNode` requires to revert the its meaning (I.e., only with this
patch, `true` should be set an option). Therefore, the following patch renames
it.
Differential Revision: https://phabricator.services.mozilla.com/D113228
Currently, its `mEditorWasDestroyed` is set to true even while `input` event
is being dispatched, i.e., even when it finished everything for the edit action.
However, the flag should be only for blocking unexpected cause which is caused
by mutation event listeners or something other unexpected event listeners
which shouldn't be run while editor is handling an edit action.
Therefore, this patch makes it store "handled" state which is marked before
notifying editor observers and dispatching `input` event and the flag won't
be set to true after it's marked as "handled".
Depends on D111579
Differential Revision: https://phabricator.services.mozilla.com/D111580
The editing session is temporarily disabled when opening a document. Then,
when the document is closed, the editing session is restored with same
`HTMLEditor` instance. Therefore, initializing editor may occur during
an edit action is being handled.
Probably, this must not be used in normal webapps since mutation events are
used rarely. So, I think that for avoiding further complicated things, we
should make editor won't accept new edit action handling after original
editing session ended.
This patch renames `AutoEditActionDataSetter::CanHandle()` to
`IsDataAvailable()` since it just checks whether the important resource is
available or not. And makes new `CanHandle()` return `false` for
non-initializing edit actions if editing session has ended.
Differential Revision: https://phabricator.services.mozilla.com/D111241
The lifetime of it is guaranteed by `AutoEditActionDataSetter` which grabs
`Selection` until it's destroyed, and it's a stack only class and created
at first step of all public method calls. Therefore, we can mark it as
`MOZ_KNOWN_LIVE` and we can change it returning reference of `Selection`
instead of reference of `RefPtr<Selection>`.
Differential Revision: https://phabricator.services.mozilla.com/D110896
Currently, it dispatches `input` event with holding `mPlaceholderTransaction`
and `mPlaceholerBatch` as 1. If `input` event listener sets value in XUL
document, the setting value transaction will break the existing placeholder
transaction. Then, only the last one will be undone, and it causes inserting
new padding `br` element with transaction and clears redo history.
This patch makes it forget them before dispatching `input` event.
Note that I tried to create automated test for this, but I cannot reproduce
this with simple testcase. I guess something other things also required to
reproduce this bug.
Differential Revision: https://phabricator.services.mozilla.com/D110715
For making delete handlers simpler, and set better target ranges to the
corresponding `beforeinput` event, we should ignore non-editable ranges
before handling deletion.
This patch makes editor stop handling deleteion when a range crosses editing
host boundaries. In this case, Gecko has done nothing, but fired
`beforeinput` event. Note that Blink deletes editable contents in the range
**until** it meets first non-editable content, but I don't think this is
a good behavior because it makes things complicated. Therefore, I filed
a spec issue: https://github.com/w3c/editing/issues/283
On the other hand, this behavior change causes different behavior in
https://searchfox.org/mozilla-central/source/editor/libeditor/crashtests/1345015.html
It tries to insert paragraph into `<html>` element, but our editor currently
does not support it. Therefore, it hits `MOZ_ASSERT`. Therefore, this patch
added a new check into `HTMLEditor::InsertParagraphSeparatorAsSubAction()`.
Differential Revision: https://phabricator.services.mozilla.com/D107588
Blink treats each non-editable node as an atomic object. E.g., deleting or
forward-deleting from next to a non-editable element, it deletes only one
non-editable element.
Unfortunately, our layout treat adjacent non-editable nodes as a node.
Therefore, the adding WPTs do not work, but they are not new regression of
this patch.
Differential Revision: https://phabricator.services.mozilla.com/D107587
Before deleting `IMEState::Enabled::PLUGIN`, let's make it an enum class
for making the change safer. Almost all of this change is done by
"replace" of VSCode.
Differential Revision: https://phabricator.services.mozilla.com/D100100
The reason of hitting the assertion is, a clipboard event listener nests
another edit action, initializing the editor, runs, and tries to create an
anonymous `<br>` element with transaction, but it checks whether `beforeinput`
event has already been dispatched or not with the parent edit action data.
For fixing it, this patch adds a flag to edit action data to indicate whether
a clipboard event has already been dispatched or not. If it's already been
dispatched, it's **okay** to modify the DOM tree. Ideally, we should put off
modifying the DOM tree after dispatching `beforeinput` event, but this case
does not notify web apps anything so that this patch should be fine.
Although this patch adds a crash test which is attached by the reporter,
it's not reproducible with current editor code because we stopped modifying
native anonymous nodes with transaction, but it depends on the behavior.
Still this assertion hits in `test_clipboard_noeditor.html` intermittently,
but I don't have idea how to reproduce this permanently. Therefore, this
patch does not contain a test to check to regression actually.
Differential Revision: https://phabricator.services.mozilla.com/D95115
Blink and WebKit do not fire `beforeinput` event when user uses build-in
password manager and autocomplete. But the `inputType` value for this case,
`"insertReplacementText"` is defined as cancelable in the spec, and it's
actually cancelable when it's fired for correcting a word with built-in
spellchecker of them.
For making only our users' autocomplete and password manager not blocked by
web apps, we should make them not cancelable by default, but I think that we
should keep dispatching such non-cancelable `beforeinput` for conforming to
the standard unless we'd get a web-compat report for this.
Differential Revision: https://phabricator.services.mozilla.com/D93206
Unfortunately, there is no official way to detect whether browser supports
or not `beforeinput` event in the wild because Chromium does not support
`onbeforeinput` event handler attribute. On the other hand, we're the
last browser vendor which does not support `beforeinput` event, and we
make `InputEvent.getTargetRanges()` enabled only when `beforeinput` event
because it returns meaningful value only when the event type is `beforeinput`.
So, web apps can detect `beforeinput` event support with checking type of
or existence of it instead. However, in our experience, it's guessed what
a lot of web apps check whether "Firefox" or not to consider whether it
can use `beforeinput` events. If so, we need to announce shipping
`beforeinput` event and the way to feature detection before actually shipping
it. Otherwise, we wouldn't get enough feedback from testers.
First of all for shipping `beforeinput` events, we should collect how much
web apps which use `HTMLEditor` use `beforeinput` event when it's enabled,
and how much web apps use mutation events or mutation observer instead of
`beforeinput` events even when it's enabled.
Honestly, I'd like to collect URLs which don't use `beforeinput` event, but
use mutation events or mutation observer. But it's really sensitive data
so that I believe that we shouldn't collect it.
Differential Revision: https://phabricator.services.mozilla.com/D92548
When `HTMLEditor` instances are destroyed, I'd like to collect how much
instances are worked with `beforeinput` event listeners. Before adding such
telemetry probe, this patch adds methods to set/get whether a `beforeinput`
event listener has had added or not to `nsPIDOMWindowInner`.
Differential Revision: https://phabricator.services.mozilla.com/D92546
This patch tries to mark root callers of `nsINode::GetSelectionRootContent()`
which calls `nsINode::GetAnonymousRootElementOfTextEditor()` as far as possible
(and reasonable).
It's used by `ContentEventHandler` so that a lot of methods of
`EventStateManager`, `ContentEventHandler`, `IMEContentObserver` which are main
users of it are also marked as `MOZ_CAN_RUN_SCRIPT`. I think that this is
reasonable.
On the other hand, it might not be reasonable to mark `IMEStateManager` methods
as `MOZ_CAN_RUN_SCRIPT` for initializing `IMEContentObserver` because
`IMEStateManager` may be able to initialize `IMEContentObserver` asynchronously
and its root callers are in XUL layout code. Therefore, this patch uses
`MOZ_CAN_RUN_SCRIPT_BOUNDARY` for `IMEStateManager` at least for now.
Differential Revision: https://phabricator.services.mozilla.com/D92730
This patch tries to mark root callers of `nsINode::GetSelectionRootContent()`
which calls `nsINode::GetAnonymousRootElementOfTextEditor()` as far as possible
(and reasonable).
It's used by `ContentEventHandler` so that a lot of methods of
`EventStateManager`, `ContentEventHandler`, `IMEContentObserver` which are main
users of it are also marked as `MOZ_CAN_RUN_SCRIPT`. I think that this is
reasonable.
On the other hand, it might not be reasonable to mark `IMEStateManager` methods
as `MOZ_CAN_RUN_SCRIPT` for initializing `IMEContentObserver` because
`IMEStateManager` may be able to initialize `IMEContentObserver` asynchronously
and its root callers are in XUL layout code. Therefore, this patch uses
`MOZ_CAN_RUN_SCRIPT_BOUNDARY` for `IMEStateManager` at least for now.
Differential Revision: https://phabricator.services.mozilla.com/D92730
`editor/libeditor/tests/test_bug974309.html` and
`editor/libeditor/tests/test_bug1268736.html` start to throw exception if
`beforeinput` event is enabled. They test XPCOM editor API when a non-editable
element has focus and it's outside of any editing hosts. In this case,
we shouldn't throw an exception for backward compatibility. Although the
all callers should stop handling any edit action in this case, let's make
`MaybeDispatchBeforeInputEvent()` return a special success code instead of
an error code for making all callers keep working with traditional paths.
Differential Revision: https://phabricator.services.mozilla.com/D91864
Currently, target ranges do not extend the ranges even when deleting empty
parent elements of the range. Therefore, the frag indicating whether empty
parents removed or not is not necessary for the methods.
Differential Revision: https://phabricator.services.mozilla.com/D90542
CLOSED TREE
Backed out changeset d3d67293f115 (bug 1623413)
Backed out changeset 75ed1b8a5c67 (bug 1623413)
Backed out changeset 0eef32d6a454 (bug 1623413)
Masayuki Nakano
Eitan Isaacson
Greg Tatum
ssummar
Cristian Tuns
Butkovits Atila
Neil Deakin
Kagami Sascha Rosylight
Ryan VanderMeulen
Csoregi Natalia
Jonathan Kew
Brindusan Cristian
Emilio Cobos Álvarez
Simon Giesecke
Narcis Beleuzu
longsonr
Mihai Alexandru Michis