It's valid thing that a container of a `Range` of `Selection` is not a content
node. Actually, it can be a `Document` node. But it's illegal case for
editor. So, if `HTMLEditor` meets such case, it does not need to do anything.
This patch makes that if `HTMLEditor` meets the situation at very first time
of public edit action method, it returns "OK" for avoiding new exception case.
Otherwise, i.e., it's an XPCOM API or meeting such situation after a DOM
mutation, returns error.
Differential Revision: https://phabricator.services.mozilla.com/D65278
--HG--
extra : moz-landing-system : lando
The key here is to test the type of the variable declaration for being a
smartptr type, instead of testing the type of the variable _use_.
Differential Revision: https://phabricator.services.mozilla.com/D65581
--HG--
extra : moz-landing-system : lando
It's valid thing that a container of a `Range` of `Selection` is not a content
node. Actually, it can be a `Document` node. But it's illegal case for
editor. So, if `HTMLEditor` meets such case, it does not need to do anything.
This patch makes that if `HTMLEditor` meets the situation at very first time
of public edit action method, it returns "OK" for avoiding new exception case.
Otherwise, i.e., it's an XPCOM API or meeting such situation after a DOM
mutation, returns error.
Differential Revision: https://phabricator.services.mozilla.com/D65278
--HG--
extra : moz-landing-system : lando
I forgot to add this check only here. (I also checked again for all similar
methods' callers.) So, if the point is end of a text node (i.e., offset equals
its length), `IsCharNBSP()` refers wrong address.
I cannot find a way to reproduce this crash, therefore, this patch does not
have new crashtest.
(Additionally, this corrects the misspell in the method name.)
Differential Revision: https://phabricator.services.mozilla.com/D65279
--HG--
extra : moz-landing-system : lando
The check was written with `NS_ASSERTION`, but I realized that it's possible
case with mutation event listeners. Therefore, I changed it to `if` and
`return`, but I forgot to revert the sign of inequality.
Differential Revision: https://phabricator.services.mozilla.com/D65280
--HG--
extra : moz-landing-system : lando
In most cases, `InputEvent.getTargetRange()` of `beforeinput` event should
return `Selection` ranges at dispatching the event.
This patch also handles special cases.
* composition change - target range should be the previous composition string
which will be replaced with new composition string.
* replace text - target range should be the replace range. This is used by
spellchecker.
* drop - target range should be the drop point.
However, the other exception is not handled by this patch. That is, deletions.
The target range(s) should be the range(s) which will be removed. In most
cases, they also matches selection ranges, but may be extended to:
* surrogate pair boundary
* grapheme cluster boundary like complex emoji
* word/line deletion deletion
* `Backspace` or `Delete` from collapsed selection
* to end of unnecessary whitespaces
For supporting these cases, we need to separate
`HTMLEditor::HandleDeleteSelection()` and its helper methods and helper class
to range computation part and modifying the DOM tree part. Of course, it
requires big changes and `InputEvent.getTargetRanges()` may be important for
feature detection of `beforeinput` event so that we should put off the big
changes to bug 1618457.
Differential Revision: https://phabricator.services.mozilla.com/D64730
--HG--
extra : moz-landing-system : lando
`InputEventOptions` should be able to take target ranges for `beforeinput`
event. However, it requires to include `StaticRange.h` from `nsContentUtils.h`
even though most `nsContentUtils.h` users don't need it. Therefore, this patch
moves it from `nsContentUtils.h` to new header file.
And makes `nsContentUtils::DispatchInputEvent()` moves the target ranges
from `InputEventOptions` to `InternalEditorInputEvent`.
Differential Revision: https://phabricator.services.mozilla.com/D64729
--HG--
extra : moz-landing-system : lando
Note that the input can be `EditorDOMPointInText`, but modified range may start
and/or end with different container. Therefore, it needs to take
`EditorDOMPoint` rather than `EditorDOMPointInText`.
Differential Revision: https://phabricator.services.mozilla.com/D64339
--HG--
extra : moz-landing-system : lando
With adding new type, `EditorDOMPointInText` whose container is
`RefPtr<dom::Text>`, we can replace `WSRunScanner::WSPoint` and make
`WSRunScanner` and `WSRunObject` can use its various API. Then, this
patch adds a lot of `NS_ASSERTION`s which can detect existing bugs.
Note that it may look like redundant that `EditorDOMPointInText::IsChar*()`
requires `EditorDOMPointInText::IsEndOfContainer()` check before that.
However, this makes what the callers check clearer.
Differential Revision: https://phabricator.services.mozilla.com/D64336
--HG--
extra : moz-landing-system : lando
Before changing `WSPoint` to `EditorDOMPointBase`, we need to change some
methods which are helped by the methods returning `WSPoint`.
Differential Revision: https://phabricator.services.mozilla.com/D64335
--HG--
extra : moz-landing-system : lando
Some script run methods of `WSRunObject` guarantee the lifetime of its
`mHTMLEditor`. However, they should be guaranteed by the instantiators
(all of them are `HTMLEditor`).
Differential Revision: https://phabricator.services.mozilla.com/D64334
--HG--
extra : moz-landing-system : lando
For making it easier to review the following patches, rename the overloads
of `WSRunScanner::Get(Previous|Next)CharPoint()` which take `WSPoint`.
Differential Revision: https://phabricator.services.mozilla.com/D64333
--HG--
extra : moz-landing-system : lando
They are fallback methods when the container of given point is not a text node
in `WSRunScanner::mNodeArray`, and then, looks for a text node for the given
point.
Differential Revision: https://phabricator.services.mozilla.com/D64332
--HG--
extra : moz-landing-system : lando
Some methods of `EditorDOMPointBase` assumes the container node type is
`nsINode`. However, it's not good for reuse. Therefore, this patch makes
most methods of them be template methods.
Differential Revision: https://phabricator.services.mozilla.com/D64331
--HG--
extra : moz-landing-system : lando
With the preceding patches, `HTMLEditor` mostly does not need to be a friend of
`WSRunObject`.
This patch stops it, and adding some self-documented methods for checking
`mStartReason` and `mEndReason`, and also adding self-documented alternative
methods of `GetStartReasonContent()` and `GetEndReasonContent()`.
Differential Revision: https://phabricator.services.mozilla.com/D63616
--HG--
extra : moz-landing-system : lando
When `WSScanResult::ReachedCurrentBlockBoundary()` returns true, reached content
of backward scan result is same as the scanner's `GetStartReasonContent()` and
reached content of forward scan result is same as the scanner's
`GetEndReasonContent()`. For making code in the blocks of
`if (foo.ReachedCurrentBlockBoundary())` easier to understand, we should use
the result's content.
Differential Revision: https://phabricator.services.mozilla.com/D63615
--HG--
extra : moz-landing-system : lando
They are really messy because they take a lot of out parameters, and these
out parameter meaning is really unclear. Therefore, they should return
a stack only class instance which explain the meaning with getter methods.
And also it should store the result node as `nsIContent`.
And also this patch adds static methods for them for their users which don't
need `WSRunScanner` instance.
Differential Revision: https://phabricator.services.mozilla.com/D63613
--HG--
extra : moz-landing-system : lando
Now, all setter guarantee that they are subclass instances of `nsIContent`.
Differential Revision: https://phabricator.services.mozilla.com/D63612
--HG--
extra : moz-landing-system : lando
Its result has 4 meanings:
1. an editable block element for container of `mScanStartPoint`.
2. a topmost inline editable content for container of `mScanStartPoint` if there
is no editable block parent.
3. container of `mScanStartPoint` if it's a block (either editable or
non-editable).
4. container of `mScanStartPoint` if its parent is not editable and a inline
content.
#1, #2 and editable case of #3 make sense because the results are topmost
editable content in current context. On the other hand, non-editable case
of #3 and #4 are caused by unexpected wrong fallback code.
So, let's make it always returns the content in the former meaning and if
the caller needs the latter one, they should use the container by themselves.
Therefore, for making what's the start of the search, this patch also makes
new method take start content instead of hiding `mScanStartPoint` from the
callers.
Differential Revision: https://phabricator.services.mozilla.com/D63610
--HG--
extra : moz-landing-system : lando
This covers most cycle collected objects which support weak references, but
not the ones which inherit from a cycle collected class and don't do any cycle
collection on their own.
Differential Revision: https://phabricator.services.mozilla.com/D63962
--HG--
extra : moz-landing-system : lando
Currently, it checks whether proper element gets focus as expected with
`nsFocusManager::GetFocusedElement()`, but it returns globally focused element.
I.e., it may return different document's element or `nullptr` if application
itself is inactive.
The purpose of the focus check is, `HTMLEditor` can modify contents only in
active editing host. Therefore, comparing with
`HTMLEditor::GetActiveEditingHost()` is better and simpler for here.
Differential Revision: https://phabricator.services.mozilla.com/D63581
--HG--
extra : moz-landing-system : lando
This patch assumes that only element node can have content node. I.e., we
won't hit the following `MOZ_ASSERT`:
```
Element* element = nullptr;
nsIContent* content = aContent;
while (content) {
if (content->IsElement()) {
element = content->AsElement();
break;
}
content = content->GetParent();
}
MOZ_ASSERT(!content || content == element || content->GetParent() == element);
```
Differential Revision: https://phabricator.services.mozilla.com/D63308
--HG--
extra : moz-landing-system : lando
This removes the need for explicit #ifdef NS_BUILD_REFCNT_LOGGING without
introducing user-defined destructors when it is not defined.
Also, some uses of virtual for declaring destructors are replaced by the
appropriate override declaration through these changes.
Differential Revision: https://phabricator.services.mozilla.com/D62604
--HG--
extra : moz-landing-system : lando
This removes the need for explicit #ifdef NS_BUILD_REFCNT_LOGGING without
introducing user-defined destructors when it is not defined.
Also, some uses of virtual for declaring destructors are replaced by the
appropriate override declaration through these changes.
Differential Revision: https://phabricator.services.mozilla.com/D62604
--HG--
extra : moz-landing-system : lando
IsEmptyNode won't return error if parameter is not null. So we shouldn't use
nserror for return value and use assertion to check parameter.
Differential Revision: https://phabricator.services.mozilla.com/D63124
--HG--
extra : moz-landing-system : lando
Chrome does not allow nested `Document.execCommand()` calls:
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/editing/commands/document_exec_command.cc;l=75;drc=301e5d079a1b4c29c5b17574d0470e6db7370acc
On the other hand, Safari (and Firefox) allows it. However, it's worthwhile to
follow Chrome's behavior.
This patch makes `Document::ExecCommand()` return `false` when it's called
while running another `Document::ExecCommand()` call on Nightly and early Beta.
This is exactly same behavior, and we should watch broken web apps reports
for a while before riding this on the train.
And this patch sets the pref to `true` when all crash tests under
`editor/libeditor/crashtests` which depend on nested calls of `execCommand` run
since same things may be reproducible with other DOM APIs.
Differential Revision: https://phabricator.services.mozilla.com/D62815
--HG--
extra : moz-landing-system : lando
The timeout is caused by that `iframe.contentWindow` may not receive `click`
event (as far as I've tested, neither `mousedown` nor `mouseup` is fired in
that case) when synthesizing a mouse click over the `<iframe>` element with
parent window. However, if it synthesizes mouse click with `<span>` element
in the `<iframe>` and `iframe.contentWindow`, `click` event is always fired.
Differential Revision: https://phabricator.services.mozilla.com/D62992
--HG--
extra : moz-landing-system : lando
Finally, this patch makes `HTMLEditor::DoInsertHTMLWithContext()` stop using
array of `nsINode`.
Differential Revision: https://phabricator.services.mozilla.com/D61984
--HG--
extra : moz-landing-system : lando
For guaranteeing the meaning of `aArrayOfListAndTableRelatedElements`,
`ReplaceOrphanedStructure()` should call
`CollectListAndTableRelatedElementsAt()` by itself rather than taking as
a parameter.
Then, what it does becomes a little bit clearer. Therefore, this patch renames
`ReplaceOrphanedStructure()` to `EnsureBeginsOrEndsWithValidContent()` and
`DiscoverPartialListsAndTables()` to `GetMostAncestorListOrTableElement()`,
and adds a lot of comments.
As far as I've tested by my hand, the behavior is not changed even though
it's really buggy in some cases. We should fix them after writing automated
tests in another bug.
Differential Revision: https://phabricator.services.mozilla.com/D61983
--HG--
extra : moz-landing-system : lando
For guaranteeing the meaning of list or `<table>` element in
`ReplaceOrphanedStructure()`, it should call `DiscoverPartialListsAndTables()`.
Then, their meaning becomes clearer than coming from its parameter.
Differential Revision: https://phabricator.services.mozilla.com/D61982
--HG--
extra : moz-landing-system : lando
In these patches, we know that there are a lot of helper methods to fix node
list which is created by `SubtreeContentIterator` and will be inserted into
the editor's DOM tree, and they are not used by other places. So, we can
encapsulate them into a stack only class for making their usage clearer.
Differential Revision: https://phabricator.services.mozilla.com/D61981
--HG--
extra : moz-landing-system : lando
We should port `editor/libeditor/tests/test_bug622371.html` to WPT. It was
written for compatibility issue with CKEditor 9 years ago and Gecko and
Blink behave as exactly same. Only Safari normalize the selection into the
end of the text node, but Safari also does not change selection at turning
on/off `designMode` too. Therefore, all browser engines have implicit
agreement at least that selection shouldn't be changed at changing
`designMode`.
Differential Revision: https://phabricator.services.mozilla.com/D62813
--HG--
extra : moz-landing-system : lando
Unfortunately, even with this cleaning up, I don't understand what it does
because existing comment and what actually it does seem different.
Differential Revision: https://phabricator.services.mozilla.com/D61980
--HG--
extra : moz-landing-system : lando
`HTMLEditor::DiscoverPartialListsAndTables()` can be static and we can make
its definition simpler. However, due to the odd logic, I'm still not sure
what it does. I'll update it after cleaning up its result user,
`HTMLEditor::ReplaceOrphanedStructure()`.
Differential Revision: https://phabricator.services.mozilla.com/D61979
--HG--
extra : moz-landing-system : lando
`HTMLEditor::CreateListOfNodesToPaste()` can be static and it does not need to
take `DocumentFragment` as an argument if the range is always specified.
For avoiding 2 input path to specify a range, we should make it take only
range boundaries and array of nodes to return the collected nodes.
Differential Revision: https://phabricator.services.mozilla.com/D61978
--HG--
extra : moz-landing-system : lando
Masayuki Nakano
Mark Banner
Boris Zbarsky
Mirko Brodesser
Coroiu Cristina
Simon Giesecke
Kris Maglione
Sylvestre Ledru
Dorel Luca
Makoto Kato