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
* getAttributeValue
* cloneAttribute
* splitNode
* joinNodes
* markNodeDirty
* removeEditorObserver
are not used from script. Therefore, we can get rid of them from `nsIEditor`.
However, `EditorBase::GetAttributeValue()` is referred by
`HTMLEditor::CopyCellBackgroundColor()` and it's just a wrapper of
`Element::GetAttr()`. Therefore, this patch makes
`HTMLEditor::CopyCellBackgroundColor()` use `Element::GetAttr()` directly.
And also `EditorBase::MarkNodeDirty()` is used from some friend classes.
Therefore, this patch redesigns it as `MarkElementDirty()` and make all of
them check whether the method call destroys the editor.
Differential Revision: https://phabricator.services.mozilla.com/D60796
--HG--
extra : moz-landing-system : lando
Current `nsIHTMLEditor::GetSelectedElement()` just checks whether the
found element is followed by `<br>` element or not. However, following
element may start with `<br>` element and if there is end of the range,
`PostContentIterator` does not list up the `<br>` element.
Therefore, it should check whether the found element is followed by
`<br>` element which is starts the next sibling element too.
Differential Revision: https://phabricator.services.mozilla.com/D60385
--HG--
extra : moz-landing-system : lando
Simon Giesecke
Dorel Luca
Makoto Kato
Mirko Brodesser
Masayuki Nakano
shindli
Emilio Cobos Álvarez
Boris Zbarsky
Mark Banner
Mihai Alexandru Michis