Only `TypeInState` stores last caret position. So, only it can detect the
case caret is moved from outside of `<a href>` element and is now at start
or end of it.
Note that this patch does not assume `<a href>` has an empty text node with
another inline element. If we supported it, the loop would be more complicated
and it's really unrealistic edge case. Therefore, it's reasonable to ignore
the case.
And also this patch works with `ArrowUp`/`ArrowDown`/`PageUp`/`PageDown`
cases. However, I have no idea to reject such cases. But I guess that
it does not so bad behavior for users because caret does not moved in
a text node in `<a href>`.
Depends on D69479
Differential Revision: https://phabricator.services.mozilla.com/D69480
--HG--
extra : moz-landing-system : lando
The selection validation should be checked in the constructor of
`ParagraphStateAtSelection` because
`EditorBase::IsSelectionRangeContainerNotContent()` needs edit action data.
Additionally, this patch adds a test for `nsIHTMLEditor.getParagraphState()`.
Differential Revision: https://phabricator.services.mozilla.com/D69316
--HG--
extra : moz-landing-system : lando
I'm still not sure how the crash occurs especially on Thunderbird. However,
it could be possible if `pointToInsert` is modified with an orphan node (i.e.,
when `pointToInsert.GetContainer()` returns `nullptr`). Therefore, this patch
makes it check whether the inserted node stays at expected position or not,
and if it's not, make it keep inserting next content nodes to previous position
because it must look like odd that inserting to different position.
Differential Revision: https://phabricator.services.mozilla.com/D69154
--HG--
extra : moz-landing-system : lando
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's __attribute__((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Differential Revision: https://phabricator.services.mozilla.com/D69315
--HG--
extra : moz-landing-system : lando
`AlignStateAtSelection` class is instantiated outside of editor class so that
we shouldn't make each user guarantee that there is selection range
(fortunately, the putting off cost is really low).
And as far as I tested, Blink and WebKit does not throw exception when
`Document.qeuryCommand*("justify*")` is called without selection ranges.
So, this patch also prevents exception in this situation.
Differential Revision: https://phabricator.services.mozilla.com/D68755
--HG--
extra : moz-landing-system : lando
Now, `WSType` is used only by `WSRunScanner`, `WSRunObject` and `WSScanResult`.
We should hide this mysterious `enum` from other classes for making other
developers easier to understand. Therefore, this patch moves `WSType` into
`WSScanResult` and share it with `WSRunScanner` with making them friends.
Depends on D68675
Differential Revision: https://phabricator.services.mozilla.com/D68676
--HG--
extra : moz-landing-system : lando
Surprisingly, each value meaning of `WSFragment::mType` is different from
`WSFragment::mRightType` and `WSFragment::mLeftType`. It uses only 3 bits,
`WSType::normalWS`, `WSType::leadingWS` and `WSType::trailingWS`.
According to the second `if` block of `WSRunScanner::GetRuns()`, the latter
2 values mean whether the fragment starts from and/or ends by a hard line.
And also, according to there, `normalWS` means the run is visible or not.
(However, according to the first `if` block, `Visible::Yes` might be set to
for empty fragment too, but I have no better idea of its name.)
Therefore, `mType & WSType::leadningWS` can call "is start of hard line",
`mType & WSType::trailingWS` can call "is end of hard line",
`mType == WSType::normalWS` can call "is visible and not edges of hard line"
and `mType & WSType::normalWS` can call "is not edges of hard line".
So, 3 `bool` members can represent all of their status. Therefore, we should
get rid of this odd use case of `WSType`.
Depends on D68674
Differential Revision: https://phabricator.services.mozilla.com/D68675
--HG--
extra : moz-landing-system : lando
Similarly, it indicates the previous content type of the fragment.
Depends on D68673
Differential Revision: https://phabricator.services.mozilla.com/D68674
--HG--
extra : moz-landing-system : lando
It means why the `WSFragment` ends by. I.e., it tells next content type of
the fragment.
Differential Revision: https://phabricator.services.mozilla.com/D68673
--HG--
extra : moz-landing-system : lando
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's __attribute__((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Differential Revision: https://phabricator.services.mozilla.com/D68747
--HG--
extra : moz-landing-system : lando
I give up to write a clean patch for this bug with current design. The trigger
is indeed bug 1618089, but this is a hidden regression of bug 1530649.
Starting from bug 1530649, `WSRunObject` started to use `EditorDOMPoint` for
storing the specified point. And it may store (or only store) child node.
Therefore, if it points a text node and it's removed by
`WSRunObject::DeleteRange()`, the point becomes invalid even if its offset
is still valid. Therefore, we should make `mStartScanPoint` and `mEndScanPoint`
forget their child before DOM tree change ideally, but it means that we need
to compute offset of the child every time before changing the DOM tree. We
cannot accept this safest approach due to performance reason.
Therefore, this patch just invalidates `mStartScanPoint`'s child node only when
it's reused after the DOM tree is modified.
Differential Revision: https://phabricator.services.mozilla.com/D68048
--HG--
extra : moz-landing-system : lando
This rejiggers a bit the way selection focus is handled so that focusing a
disabled form control with the mouse handles selection properly, and hides the
document selection and so on.
This matches the behavior of other browsers as far as I can tell.
Given now readonly and disabled editors behave the same, we can simplify a bit
the surrounding editor code.
Differential Revision: https://phabricator.services.mozilla.com/D66464
--HG--
extra : moz-landing-system : lando
Helps to determine which methods of `nsFrameSelection` are intended to
be called only for the `eNormal` Selection.
Driven by the idea that state of `nsFrameSelection` relevant only for
the `eNormal` Selection should later move to a `NormalSelection` class.
Differential Revision: https://phabricator.services.mozilla.com/D67445
--HG--
extra : moz-landing-system : lando
This never worked, but it's more visible with the new form controls which have
more padding.
Make the anonymous div and co a pseudo-element, so that they inherit from the
<input> properly in all cases. This works for non-number inputs because the
editor root is a direct child of the <input>, but it doesn't for number inputs
because there's a flex wrapper in between.
This way overflow-clip-box: inherit does what we want. Reset the padding in the
inline direction, as the padding for <input type=number> applies to the arrow
boxes as well, and thus we'd double-apply it.
Differential Revision: https://phabricator.services.mozilla.com/D65271
--HG--
extra : moz-landing-system : lando
Actually GeckoView turns off spellchekcer, so it is unnecessary to run reftests
for spellchecker on GeckoView.
Differential Revision: https://phabricator.services.mozilla.com/D66540
--HG--
extra : moz-landing-system : lando
This rejiggers a bit the way selection focus is handled so that focusing a
disabled form control with the mouse handles selection properly, and hides the
document selection and so on.
This matches the behavior of other browsers as far as I can tell.
Given now readonly and disabled editors behave the same, we can simplify a bit
the surrounding editor code.
Differential Revision: https://phabricator.services.mozilla.com/D66464
--HG--
extra : moz-landing-system : lando
Additionally, this makes some `nsIAbsorbingTransaction` methods infallible for
reducing unnecessary warnings.
Differential Revision: https://phabricator.services.mozilla.com/D66179
--HG--
extra : moz-landing-system : lando
This also changes `mChildren` to array of `OwningNonNull` for removing
unnecessary `nullptr` checks.
Depends on D65874
Differential Revision: https://phabricator.services.mozilla.com/D65875
--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
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
It can be a static method and does not make sense taking array of nodes as
input argument because it refers only first or last node of the array.
Differential Revision: https://phabricator.services.mozilla.com/D61977
--HG--
extra : moz-landing-system : lando
The name does not explain what it does. Additionally, it should be same style
as `HTMLEditor::IsReplaceableListElement()` for making it easier to understand.
Differential Revision: https://phabricator.services.mozilla.com/D61976
--HG--
extra : moz-landing-system : lando
`HTMLEditor::ScanForListAndTableStructure()` is complicated because it has
2 modes, one is for handling list element, the other is for handling table
element. This patch splits them as 2 methods.
Differential Revision: https://phabricator.services.mozilla.com/D61975
--HG--
extra : moz-landing-system : lando
Helper methods of `HTMLEditor::DoInsertHTMLWithContext()` which uses array of
`nsINode` is really messy and we can make them simpler. This is first
preparation of a series of refactoring patches of them.
Differential Revision: https://phabricator.services.mozilla.com/D61974
--HG--
extra : moz-landing-system : lando
`HTMLEditor::CollapseAdjacentTextNodes()` collects editable text nodes first so
that the array can be array of `dom::Text`. Additionally, using
`DOMSubtreeIterator` instead of `ContentSubtreeIterator` makes the code easier
to understand.
Differential Revision: https://phabricator.services.mozilla.com/D61973
--HG--
extra : moz-landing-system : lando
Same as the previous patch, the content iterator won't return non-`nsIContent`
node. Therefore, `HTMLEditor::HandleDeleteNonCollapsedSelection()` can treat
it with array of `nsIContent`.
Differential Revision: https://phabricator.services.mozilla.com/D61972
--HG--
extra : moz-landing-system : lando
The content iterators always return `nsIContent` even though the result of
`GetCurrentNode()` is `nsINode*`. Therefore, we can make
`HTMLEditor::RemoveEmptyNodesIn()` use array of `nsIContent` instead of
array of `nsINode`.
Differential Revision: https://phabricator.services.mozilla.com/D61971
--HG--
extra : moz-landing-system : lando
Unless editor needs to treat document node, all nodes are sub-class of
`nsIContent`. Therefore, we can make `HTMLEditor::CollectEditTargetNodes()`
return array of `nsIContent` instead of array of `nsINode`.
Differential Revision: https://phabricator.services.mozilla.com/D61970
--HG--
extra : moz-landing-system : lando
This patch creates `DOMIterator::AppendNodesToArray()` which takes pointer to
a functor and removes `DOMIterator::AppendList()`. This allows callers to use
lambda rather than the remaining functors, `UniqueFunctor` and
`EmptyEditableFunctor`, which are used only once. Therefore, writing them as
lambda makes the callers easier to understand. Finally, we can remove those
functor classes' base class, `BoolDomIterFunctor` too.
Note that this patch avoids using `std::function` even though if we'd use it
as functor, we could make each lambda can capture variables and each lambda
does not need to convert to point of a function with `+` operator explicitly.
The reason is, `std::function` is too slow if `AppendNodesToArray()` is called
in a hot path.
Depends on D61968
Differential Revision: https://phabricator.services.mozilla.com/D61969
--HG--
extra : moz-landing-system : lando
`TrivialFunctor` returns always true for `DOMIterator::AppendList()`. That
means that `DOMIterator` appends all nodes which are listed up by
`ContentIterator`. So, it's reasonable to make `DOMIterator` have
`AppendAllNodesToArray()` because it's more self-documented and faster.
Additionally, `BRNodeFunctor` always returns true when nodes with which
`HTMLBRElement::FromNode()` returns non-nullptr. So, we can make
`AppendAllNodesToArray()` a template class which just checks whether
every nodes are specific node type.
Differential Revision: https://phabricator.services.mozilla.com/D61968
--HG--
extra : moz-landing-system : lando
Now, we have `SpecialPowers` to use `DOMWindowUtils`. Therefore,
`test_bug1151186.html` can be a mochitest-plain and it's better since the
test checks behavior on web apps.
Depends on D62396
Differential Revision: https://phabricator.services.mozilla.com/D62397
--HG--
rename : widget/tests/test_bug1151186.html => editor/libeditor/tests/test_bug1151186.html
extra : moz-landing-system : lando
There is no meaningful listener of this so that we can get rid of it.
Depends on D61357
Differential Revision: https://phabricator.services.mozilla.com/D61358
--HG--
extra : moz-landing-system : lando
This patch makes `EditorBase::NotifyDocumentListeners()` notify
`ComposerCommandsUpdater` via `HTMLEditor::mComposerCommandsUpdater` directly.
Therefore, `ComposerCommandsUpdater` can stop inheriting
`nsIDocumentStateListener`.
Note that this patch also makes `ComposerCommandsUpdater::UpdateCommandGroup()`
not take `nsAString` as its parameter because inlinning the
`nsIDocumentStateListener` requires `ComposerCommandsUpdater.h` to include
`nsAString.h`, but it's redundant and `UpdateCommandGroup()` just compares
it with literal strings. Therefore, using `enum class` for specifying command
group is faster.
Differential Revision: https://phabricator.services.mozilla.com/D61357
--HG--
extra : moz-landing-system : lando
I thought this would fix <input type=number style="user-select: none">, but
turns out it doesn't.
<input type=number> doesn't have the editor root as a root of the anonymous
subtree, so the current hack wouldn't work, as the anon root wouldn't have the
editable flag. So tweak the code a bit to handle stuff in a simpler way than
setting the flags after the fact, and set the NAC-root flag earlier to avoid
the mOuterWrapper->AppendChildTo(root) call forgetting about root's flags.
I had to tweak one AccessibleCaret test, but that's because it uses <textarea>
with user-select: none, and our behavior there is not particularly sane. It just
happened to work because that test-case also had a bunch of contenteditable
elements, and we stop matching this rule:
https://searchfox.org/mozilla-central/rev/220a3bd6063fcbe5ca50e88dcabdc7dee0aca448/layout/style/contenteditable.css#22
Because the anonymous div now properly matches :-moz-read-write, which made the
rule apply and the test work. See comment 4 of this bug.
I'll fix this stuff up and add some tests for our behavior here in bug 1611699.
I refactored the dragdrop tests to cover more input types, but I ended up not
being able to use them because they're dependent on the content.
Instead I added an extra test and changed the refactor so that it applies to
<input type=search>, as there's layout work going on in bug 558594, and it'd be
unfortunate to regress this there too.
Differential Revision: https://phabricator.services.mozilla.com/D61094
--HG--
extra : moz-landing-system : lando
Currently, `AutoStyleCacheArray` has style information of all 19 styles.
However, its element type, `StyleCache` has `nsString` instance. Therefore,
the initializing cost is not reasonable since in most cases most of the
`StyleCache` instances are not used actually.
This patch makes `HTMLEditor::GetInlineStyles()` append elements only for
applied styles and `HTMLEditor::ReapplyCachedStyles()` look for current
value from an instance with new `IndexOf()` method. Therefore, this patch
may make slower if a lot of styles are applied. However, we can expect that
it's rare case.
Differential Revision: https://phabricator.services.mozilla.com/D61262
--HG--
extra : moz-landing-system : lando
Previously, I added `Selection::mCachedRange` to save allocation cost of
`nsRange`. However, with the previous patch, we don't need the hack anymore
since ranges removed by `Selection::RemoveAllRanges()` are always kept in
the global cache of `nsRange`. Therefore, we can remove the ugly hack right
now.
Differential Revision: https://phabricator.services.mozilla.com/D61239
--HG--
extra : moz-landing-system : lando
`nsRange` instances are allocated a lot in the heap especially by editor and
spellchecker. The allocation cost is too bad for benchmarks. Therefore,
we should reuse released instances as far as possible. For managing it in
static factory methods of `nsRange`, we need to hide `nsRange` constructor.
Differential Revision: https://phabricator.services.mozilla.com/D61237
--HG--
extra : moz-landing-system : lando
* `nsIHTMLEditor.removeAllInlineProperties`
* `nsIHTMLEditor.increaseFontSize`
* `nsIHTMLEditor.decreaseFontSize`
* `nsIHTMLEditor.setParagraphFormat`
* `nsIHTMLEditor.getBackgroundColorState`
* `nsIHTMLEditor.indent`
* `nsIHTMLEditor.align`
* `nsIEditorStyleSheets.replaceOverrideStyleSheet`
* `nsITableEditor.selectBlockOfCells`
These methods are not used by any Gecko products including comm-central and
BlueGriffon so that we should remove them. Note that only
`HTMLEditor::GetBackgroundColorState()` is used internally so that we need to
keep it as a public method of `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D61139
--HG--
extra : moz-landing-system : lando
`DragEvent::GetRangeParentContentAndOffset()` may return `nullptr`. Previously,
it was checked and returned `NS_ERROR_FAILURE` before landing bug 1610264.
This patch adds the null check which was accidentally removed.
Differential Revision: https://phabricator.services.mozilla.com/D61208
--HG--
extra : moz-landing-system : lando
`drop` event shouldn't be fired when drop target does not accept the data
at last `dragover` handling.
This patch makes `synthesizePlainDragAndDrop()` stop dispatching `drop` event
if last `dragover` event's `dataTransfer.dropEffect` is `none`. In strictly
speaking, it should refer `nsIDragSession::canDrop`. However, the value is
unstable only on Linux. The reason must be that only GTK widget manages
`canDrop` state by itself. Therefore, this patch directly uses the
`dataTransfer.dropEffect` value instead.
Differential Revision: https://phabricator.services.mozilla.com/D60476
--HG--
extra : moz-landing-system : lando
Currently, editor does not consume `dragenter`, `dragover` nor `drop` event if
`EditorEventListener::CanDrop()` returns false. Then,
`EventStateManager::PostHandleEvent()` does not modify current drag session
with given drop effect value. Therefore, it does not make sense that
`EditorEventListener` modifies modify drop effect of `DragEvent::mDataTransfer`
only with calling `Event::StopPropagation()`.
This patch makes `EditorEventListener` consume `eDragEnter`, `eDragOver` and
`eDrop` events unless drop target is not managed by the editor (i.e., when
drop target is non-editable nodes in HTML editor, or the editor is a part of
`<input type="file">`).
Then, `EditorEventListenr::DragOver()` and `EditorEventListener::Drop()`
become really similar. Therefore, this patch also merges them. Their
differences are only:
* Only `Drop()` calls `CleanupDragDropCaret()` first.
* The main purpose code (calling `TextEditor::OnDrop()` from `Drop()` and
modifying drop effect and caret from `DragOver()`)
Differential Revision: https://phabricator.services.mozilla.com/D60475
--HG--
extra : moz-landing-system : lando
For preparing to remove `nsIPlaintextEditor` interface, this patch moves
all of them to `nsIEditor`, but for avoiding bustage in comm-central, makes
`nsIPlaintextEditor` inherit `nsIEditor` for now (i.e., even with this patch,
script can access old `nsIPlaintextEditor` members with the interface.
In C++ code, this patch moves `SetWrapColumn()`, `InsertTextAsAction()`,
`InsertTextAsSubAction()` and `InsertLineBreakAsSubAction()` because
they do common things between `TextEditor` and `HTMLEditor`. On the other
hand, this does not move `TextEditor::GetTextLength()` because it's designed
only for `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D60820
--HG--
rename : editor/libeditor/tests/test_nsIPlaintextEditor_insertLineBreak.html => editor/libeditor/tests/test_nsIEditor_insertLineBreak.html
extra : moz-landing-system : lando
Masayuki Nakano
Mihai Alexandru Michis
Chris Peterson
Arthur Iakab
Emilio Cobos Álvarez
Cosmin Sabou
Simon Giesecke
Dan Glastonbury
Narcis Beleuzu
Mirko Brodesser
Kagami Sascha Rosylight
Sean Feng
Makoto Kato
Ciure Andrei
Ehsan Akhgari
Bogdan Tara
Mark Banner
Boris Zbarsky
Coroiu Cristina
Kris Maglione
Sylvestre Ledru
Dorel Luca
shindli