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
This was done by:
This was done by applying:
```
diff --git a/python/mozbuild/mozbuild/code-analysis/mach_commands.py b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
index 789affde7bbf..fe33c4c7d4d1 100644
--- a/python/mozbuild/mozbuild/code-analysis/mach_commands.py
+++ b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
@@ -2007,7 +2007,7 @@ class StaticAnalysis(MachCommandBase):
from subprocess import Popen, PIPE, check_output, CalledProcessError
diff_process = Popen(self._get_clang_format_diff_command(commit), stdout=PIPE)
- args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format]
+ args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format, '-sort-includes']
if not output_file:
args.append("-i")
```
Then running `./mach clang-format -c <commit-hash>`
Then undoing that patch.
Then running check_spidermonkey_style.py --fixup
Then running `./mach clang-format`
I had to fix four things:
* I needed to move <utility> back down in GuardObjects.h because I was hitting
obscure problems with our system include wrappers like this:
0:03.94 /usr/include/stdlib.h:550:14: error: exception specification in declaration does not match previous declaration
0:03.94 extern void *realloc (void *__ptr, size_t __size)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/malloc_decls.h:53:1: note: previous declaration is here
0:03.94 MALLOC_DECL(realloc, void*, void*, size_t)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/mozalloc.h:22:32: note: expanded from macro 'MALLOC_DECL'
0:03.94 MOZ_MEMORY_API return_type name##_impl(__VA_ARGS__);
0:03.94 ^
0:03.94 <scratch space>:178:1: note: expanded from here
0:03.94 realloc_impl
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozmemory_wrap.h:142:41: note: expanded from macro 'realloc_impl'
0:03.94 #define realloc_impl mozmem_malloc_impl(realloc)
Which I really didn't feel like digging into.
* I had to restore the order of TrustOverrideUtils.h and related files in nss
because the .inc files depend on TrustOverrideUtils.h being included earlier.
* I had to add a missing include to RollingNumber.h
* Also had to partially restore include order in JsepSessionImpl.cpp to avoid
some -WError issues due to some static inline functions being defined in a
header but not used in the rest of the compilation unit.
Differential Revision: https://phabricator.services.mozilla.com/D60327
--HG--
extra : moz-landing-system : lando
rg -l 'mozilla/Move.h' | xargs sed -i 's/#include "mozilla\/Move.h"/#include <utility>/g'
Further manual fixups and cleanups to the include order incoming.
Differential Revision: https://phabricator.services.mozilla.com/D60323
--HG--
extra : moz-landing-system : lando
In C++ code, `DataTransfer::GetTypes()` are used for checking whether the
`DataTransfer` instance has specific type `DataTransferItem` or not. Therefore,
it does not make sense to retrieve all item types nor compare some types
looking for with the retrieved item types.
This patch adds `DataTransfer::HasType()` and `DataTransfer::HasFile()` for
the current C++ users. They don't take `CallerType` since all C++ users use
`GetTypes()` as `CallertType::System`. And they just call a corresponding
method of `DataTransferItemList`.
Then, `DataTransferItemList` methods compares given type with every items
simply.
Note that this patch moves `DataTransfer::GetTypes()` to `DataTransferItemList`
too because new methods and `GetTypes()` should be maintained at every logic
changes.
The reason why there is no `DataTransfer::HasAnyOfTypes()` method is,
`DataTransfer.h` cannot include `DataTransferItemList.h` due to their
dependency but parameter pack requires inline methods.
Differential Revision: https://phabricator.services.mozilla.com/D60387
--HG--
extra : moz-landing-system : lando
When file is dropped on `contenteditable` element, the data is `BlobImpl`
instance in e10s mode. Then, editor tries to insert asynchronously with its
`BlobReader`. However, currently, `BlobReader` does not have a reference of
`DataTransfer` object for setting `beforeinput` event and `input` event.
Therefore, when you drop file into `contenteditable` element on debug build,
you see hitting `MOZ_ASSERT` because of unexpected null `DataTransfer` object.
This patch makes it store `DataTransfer` object for both `beforeinput` event
and `input` event, and dispatch `beforeinput` event only when it hasn't been
dispatched yet (i.e., not dispatched before it's created). The case shouldn't
occur, but let's have the fallback path in release builds for avoiding
inconvenience of our users.
Differential Revision: https://phabricator.services.mozilla.com/D60238
--HG--
extra : moz-landing-system : lando
It removes a script blocker. Therefore, although it depends on the caller
whether it causes running script or not. However, we should mark it as
`MOZ_CAN_RUN_SCRIPT` for safer code.
It's called only by the destructor of `nsAutoCauseReflowNotifier`. Therefore,
this patch also marks its constructor as `MOZ_CAN_RUN_SCRIPT` for making
each creator method marked as `MOZ_CAN_RUN_SCRIPT` or
`MOZ_CAN_RUN_SCRIPT_BOUNDARY`.
Most of the creators is mutation listener methods. However, `PresShell`
does nothing after destroying `nsAutoCauseReflowNotifier`. Therefore,
this patch does not change the callers in MutationObserver.cpp to use
`RefPtr<PresShell>` at calling them because changing it may cause performance
regression.
Perhaps, we should create another methods of `WillCauseReflow()` and
`DidCauseReflow()` to avoid unnecessary `MOZ_CAN_RUN_SCRIPT` marking.
However, I'm not sure whether most callers may run script or not because
of outside of my knowledge.
Differential Revision: https://phabricator.services.mozilla.com/D55805
--HG--
extra : moz-landing-system : lando
`UIEvent::GetRangeParent()` retrieves `nsIContent` instance but it needs to
return `already_AddRefed<nsINode>` because of a WebIDL method. However,
`nsIContent` is better type in C++ code. Therefore, this patch renames it
to `UIEvent::GetRangeParentContent()` and makes new `UIEvent::GetRangeParent()`
and just call it.
Additionally, some callers call `UIEvent::RangeOffset()` too, but that means
that they compute same things twice because both of them use
`nsLayoutUtils::GetContainerAndOffsetAtEvent()` with same input arguments.
Thus, `UIEvent::GetRangeParentContent()` should also return offset with optional
out argument. (Note that this does not make `RangeOffset()` use
`GetRangeParentContent()` because using out parameter for range parent causes
unnecessary computation cost for `RangeOffset()`.)
Therefore, finally, `UIEvent::GetRangeParentContent()` becomes also an alias of
raw method `UIEvent::GetRangeParentContentAndOffset()` which also returns offset
with out argument.
Differential Revision: https://phabricator.services.mozilla.com/D60095
--HG--
extra : moz-landing-system : lando
Instead, subclass nsTextControlFrame. This simplifies the code and avoids
correctness issues.
I kept the localization functionality though it is not spec compliant. But I
filed a bug to remove it in a followup.
Differential Revision: https://phabricator.services.mozilla.com/D57193
--HG--
extra : moz-landing-system : lando
Instead, subclass nsTextControlFrame. This simplifies the code and avoids
correctness issues.
I kept the localization functionality though it is not spec compliant. But I
filed a bug to remove it in a followup.
Differential Revision: https://phabricator.services.mozilla.com/D57193
--HG--
extra : moz-landing-system : lando
`AutoEditActionDataSetter` is created in the stack when editor's public method
is called and that guarantees lifetime of global objects in editor such as
editor itself, selection controller, etc.
The dispatcher of `beforeinput` event returns `NS_ERROR_EDITOR_ACTION_CANCELED`
if an event is actually dispatched but canceled. The reason why it's an error
is, editor code must stop handling anything when any methods return error.
So, returning an error code is reasonable in editor module. But when it's
filtered by `EditorBase::ToGenericNSResult()` at return statement of public
methods, it's converted to `NS_SUCCESS_DOM_NO_OPERATION`. This avoids throwing
new exception, but editor class users in C++ can distinguish whether each edit
action is canceled or handled. The reason why we should not throw new
exception from XPCOM API is, without taking care of each caller may break some
our UI (especially for avoiding to break comm-central). Therefore, this patch
does not make XPCOM methods return error code when `beforeinput` event is
canceled.
In most cases, immediately after creating `AutoEditActionDataSetter` is good
timing to dispatch `beforeinput` event since editor has not touched the DOM
yet. If `beforeinput` requires `data` or `dataTransfer`, methods need to
dispatch `beforeinput` event after that. Alhtough this is not a good thing
from point of view of consistency of the code. However, I have no better
idea.
Note 1: Our implementation does NOT conform to the spec about event order
between `keypress` and `beforeinput` (dispatching `beforeinput` event after
`keypress` event). However, we follow all other browsers' behavior so that it
must be safe and the spec should be updated for backward compatibility.
Spec issue: https://github.com/w3c/uievents/issues/220
Note 2: Our implementation does NOT conform to the spec about event order
between `compositionupdate` and `beforeinput`. Our behavior is same as
Safari, but different from Chrome. This might cause web-compat issues.
However, our behavior does make sense from point of view of consistency of
event spec. Additionally, at both `compositionupdate` and `beforeinput`,
composition string in editor has not been modified yet. Therefore, this
may not cause web-compat issues (and I hope so).
Spec issue: https://github.com/w3c/input-events/issues/49
Note that this patch makes editor detect bugs that `beforeinput` event hasn't
been handled yet when it dispatches `input` event or modifying `data` and
`dataTransfer` value are modified after dispatching `beforeinput` event with
`MOZ_ASSERT`s.
Differential Revision: https://phabricator.services.mozilla.com/D58127
--HG--
extra : moz-landing-system : lando
This patch makes `nsContentUtils::DispatchInputEvent()` dispatch `beforeinput`
event too. And also adds `onbeforeinput` event handler which is really
important for feature detection (although Chrome has not implemented this
attribute yet: https://bugs.chromium.org/p/chromium/issues/detail?id=947408).
However, we don't implement `InputEvent.getTargetRanges()` in this bug and
implementing `beforeinput` event may hit bugs of some web apps. Therefore,
this patch disables `beforeinput` event by default even in Nightly channel.
Differential Revision: https://phabricator.services.mozilla.com/D58125
--HG--
extra : moz-landing-system : lando
Some HTML editor command classes call `*AsAction()` methods multiple times.
That causes that user needs multiple undo/redo for a command and one command
causes multiple "input" events. For both compatibility with the other
browsers and making "beforeinput" cancelable, they should call `*AsAction()`
once. Instead, `HTMLEditor` should handle related element deletion.
This patch makes `HTMLEditor::RemoveInlinePropertyInternal()` remove multiple
elements if its caller allows, and `HTMLEditor::SetInlinePropertyAsAction()`
call `RemoveInlinePropertyInternal()` in some cases.
According to comm-central and BlueGriffon, we can make
`HTMLEditor::RemoveInlineProperty()` also removes the related methods
automatically because comm-central does same thing from script and BlueGriffon
does not use this. However, we cannot do that for
`HTMLEditor::SetInlineProperty()` because BlueGriffon may call it with any
HTML5 elements and does not expect removing elements in same block at that
time. If we needed to reduce the overhead of comm-central, we could change
only `RemoveInlineProperty()`, but it would make these APIs behavior
inconsistent.
Differential Revision: https://phabricator.services.mozilla.com/D58124
--HG--
extra : moz-landing-system : lando
This patch adds a lot of `beforeinput` event tests into existing mochitests
which test `input` events. But this does not add tests of canceling
`beforeinput` event because it requires really complicated path until
implementing `beforeinput` actually.
Note that `beforeinput` event is not fired with `Document.execCommand()`.
Therefore, this patch does not add WPT for testing `beforeinput` event.
And unfortunately, WPT cannot test most cases of the new tests.
Differential Revision: https://phabricator.services.mozilla.com/D58123
--HG--
extra : moz-landing-system : lando
`AutoEditActionDataSetter` is created in the stack when editor's public method
is called and that guarantees lifetime of global objects in editor such as
editor itself, selection controller, etc.
The dispatcher of `beforeinput` event returns `NS_ERROR_EDITOR_ACTION_CANCELED`
if an event is actually dispatched but canceled. The reason why it's an error
is, editor code must stop handling anything when any methods return error.
So, returning an error code is reasonable in editor module. But when it's
filtered by `EditorBase::ToGenericNSResult()` at return statement of public
methods, it's converted to `NS_SUCCESS_DOM_NO_OPERATION`. This avoids throwing
new exception, but editor class users in C++ can distinguish whether each edit
action is canceled or handled. The reason why we should not throw new
exception from XPCOM API is, without taking care of each caller may break some
our UI (especially for avoiding to break comm-central). Therefore, this patch
does not make XPCOM methods return error code when `beforeinput` event is
canceled.
In most cases, immediately after creating `AutoEditActionDataSetter` is good
timing to dispatch `beforeinput` event since editor has not touched the DOM
yet. If `beforeinput` requires `data` or `dataTransfer`, methods need to
dispatch `beforeinput` event after that. Alhtough this is not a good thing
from point of view of consistency of the code. However, I have no better
idea.
Note 1: Our implementation does NOT conform to the spec about event order
between `keypress` and `beforeinput` (dispatching `beforeinput` event after
`keypress` event). However, we follow all other browsers' behavior so that it
must be safe and the spec should be updated for backward compatibility.
Spec issue: https://github.com/w3c/uievents/issues/220
Note 2: Our implementation does NOT conform to the spec about event order
between `compositionupdate` and `beforeinput`. Our behavior is same as
Safari, but different from Chrome. This might cause web-compat issues.
However, our behavior does make sense from point of view of consistency of
event spec. Additionally, at both `compositionupdate` and `beforeinput`,
composition string in editor has not been modified yet. Therefore, this
may not cause web-compat issues (and I hope so).
Spec issue: https://github.com/w3c/input-events/issues/49
Note that this patch makes editor detect bugs that `beforeinput` event hasn't
been handled yet when it dispatches `input` event or modifying `data` and
`dataTransfer` value are modified after dispatching `beforeinput` event with
`MOZ_ASSERT`s.
Differential Revision: https://phabricator.services.mozilla.com/D58127
--HG--
extra : moz-landing-system : lando
This patch makes `nsContentUtils::DispatchInputEvent()` dispatch `beforeinput`
event too. And also adds `onbeforeinput` event handler which is really
important for feature detection (although Chrome has not implemented this
attribute yet: https://bugs.chromium.org/p/chromium/issues/detail?id=947408).
However, we don't implement `InputEvent.getTargetRanges()` in this bug and
implementing `beforeinput` event may hit bugs of some web apps. Therefore,
this patch disables `beforeinput` event by default even in Nightly channel.
Differential Revision: https://phabricator.services.mozilla.com/D58125
--HG--
extra : moz-landing-system : lando
Some HTML editor command classes call `*AsAction()` methods multiple times.
That causes that user needs multiple undo/redo for a command and one command
causes multiple "input" events. For both compatibility with the other
browsers and making "beforeinput" cancelable, they should call `*AsAction()`
once. Instead, `HTMLEditor` should handle related element deletion.
This patch makes `HTMLEditor::RemoveInlinePropertyInternal()` remove multiple
elements if its caller allows, and `HTMLEditor::SetInlinePropertyAsAction()`
call `RemoveInlinePropertyInternal()` in some cases.
According to comm-central and BlueGriffon, we can make
`HTMLEditor::RemoveInlineProperty()` also removes the related methods
automatically because comm-central does same thing from script and BlueGriffon
does not use this. However, we cannot do that for
`HTMLEditor::SetInlineProperty()` because BlueGriffon may call it with any
HTML5 elements and does not expect removing elements in same block at that
time. If we needed to reduce the overhead of comm-central, we could change
only `RemoveInlineProperty()`, but it would make these APIs behavior
inconsistent.
Differential Revision: https://phabricator.services.mozilla.com/D58124
--HG--
extra : moz-landing-system : lando
This patch adds a lot of `beforeinput` event tests into existing mochitests
which test `input` events. But this does not add tests of canceling
`beforeinput` event because it requires really complicated path until
implementing `beforeinput` actually.
Note that `beforeinput` event is not fired with `Document.execCommand()`.
Therefore, this patch does not add WPT for testing `beforeinput` event.
And unfortunately, WPT cannot test most cases of the new tests.
Differential Revision: https://phabricator.services.mozilla.com/D58123
--HG--
extra : moz-landing-system : lando
Now that we have UTF8String in the WebIDL, we can remove quite a few of the
conversions. Do that, and lift the remaining string conversions up as needed.
Also deindent Servo_ComputeColor while touching it.
Most of the remaining copies are because either bug 1606994, or because they're
WebIDL attributes that we still need to serialize back as UTF-16 (bug 1606995).
Differential Revision: https://phabricator.services.mozilla.com/D58687
--HG--
extra : moz-landing-system : lando
In particular, the ones where we transcode unconditionally atm (property names
and such).
There are others like cssText getters and setters which are a bit harder,
because I either need to rewrite all our serialization code to work with UTF8
(which is fine, but a lot of work), or teach webidl to have a setter that takes
UTF8String as input but returns DOMString as output (which is at best hacky).
Differential Revision: https://phabricator.services.mozilla.com/D58631
--HG--
extra : moz-landing-system : lando
test_password_per_word_operation.html uses magic number to consider
click point in `<input>` element. However, that causes permanent
failure on Ubuntu 18.04. Ideally, the click positions should be
computed dynamically. Therefore, this patch makes it use rect of
anonymous `<div>` element and rect of its child text node which can
be computed with `Range.getBoundingClientRect()`.
Differential Revision: https://phabricator.services.mozilla.com/D58193
--HG--
extra : moz-landing-system : lando
Most tests which use these APIs don't want to distinguish difference of
linebreaks (i.e., CRLF vs. CR vs. LF). And also most tests don't want to
check prefix and postfix of HTML data in clipboard on Windows.
Therefore, this patch makes them compare clipboard data with expected string
smarter. Every linebreak in clipboard data are treated as LF. Expected
HTML data is wrapped with `<html>`, `<body>` and comment nodes only on
Windows at comparing with clipboard data.
Differential Revision: https://phabricator.services.mozilla.com/D57963
--HG--
extra : moz-landing-system : lando
User can move DnD between different 2 editors. Then, deleting selection in
drag source editor should be handled by the drag source editor rather than
drop target editor. Therefore, `TextEditor::OnDrop()` should look for
editor instance for drag source node and call is "deleteByDrag" handler.
Differential Revision: https://phabricator.services.mozilla.com/D57449
--HG--
extra : moz-landing-system : lando
Chrome moves focus to dropped element or editing host containing dropped
element, but we don't do it. For compatibility with Chrome, it's better to
follow their behavior. Additionally, this fixes 2 issues. One is, when
dropping something into non-focused contenteditable element, we've failed to
initialize selection from `TextEditor::PrepareToInsertContent()` because
`pointToInsert` is outside of selection limiter if another editing host
has focus. The other is, when same case, we've failed to insert dropped
content because edit action handlers of `HTMLEditor` check whether editing
position is in active editing host.
Finally, this patch makes `TextEditor::OnDrop()` cancels to dispatch "input"
event if it fails something before trying to insert dropped content. Without
this change, `EditorBase::DispatchInputEvent()` tries to dispatch without
proper `data` or `dataTransfer` and that hits `MOZ_ASSERT` in `nsContentUtils`.
Additionally, this fixes an existing bug which `HTMLEditor` may insert `\r`
as-is if it comes from paste or drop. Otherwise, we need complicated `todo_is`
paths in `test_dragdrop.html`.
Differential Revision: https://phabricator.services.mozilla.com/D57447
--HG--
extra : moz-landing-system : lando
Currently, any default drop effect is "move" on editable content even when
the editor cannot remove the source element (e.g., outside of any editing host).
The drop effect is initialized by `nsContentUtils::SetDataTransferInEvent()`
which is called when `EditorEventListener::CanDrop()` and it returns true.
However, it requires unnecessary cost if we make `nsContentUtils` check
whether the dragging element is removable. Therefore, we should make
`EditorEventListener` overwrites it with checking whether the source node is
editable or not.
Differential Revision: https://phabricator.services.mozilla.com/D57446
--HG--
extra : moz-landing-system : lando
The test uses `synthesizeDrop()` etc. However, the testing data does not match
with actual data when users do same thing in some cases. Therefore, we should
rewrite it with `synthesizePlainDragAndDrop()`.
Differential Revision: https://phabricator.services.mozilla.com/D57426
--HG--
extra : moz-landing-system : lando
This changeset is a simple find and replace of `MOZ_FALLTHROUGH` and `[[fallthrough]]`.
Unfortunately, the MOZ_FALLTHROUGH_ASSERT macro (to assert on case fallthrough in debug builds) is still necessary after switching from [[clang::fallthrough]] to [[fallthrough]] because:
* MOZ_ASSERT(false) followed by [[fallthrough]] triggers a -Wunreachable-code warning in DEBUG builds
* but MOZ_ASSERT(false) without [[fallthrough]] triggers a -Wimplicit-fallthrough warning in NDEBUG builds.
Differential Revision: https://phabricator.services.mozilla.com/D56440
--HG--
extra : moz-landing-system : lando
In a follow-up commit a new `ComparePoints` method with cleaner
arguments and return value will be added.
Differential Revision: https://phabricator.services.mozilla.com/D55295
--HG--
extra : moz-landing-system : lando
This is generally pretty straightforward, and rewrites nearly all calls. It
skips the ones that it can detect using frame script globals like
`sendAsyncMessage`, though.
Differential Revision: https://phabricator.services.mozilla.com/D53740
--HG--
extra : moz-landing-system : lando
`HTMLEditor::NodeIsBlockStatic` and `nsHTMLElement::IsBlock` doesn't check whether element is real `Element`. So we should add more check whether element or not.
Differential Revision: https://phabricator.services.mozilla.com/D56684
--HG--
extra : moz-landing-system : lando
This is generally pretty straightforward, and rewrites nearly all calls. It
skips the ones that it can detect using frame script globals like
`sendAsyncMessage`, though.
Differential Revision: https://phabricator.services.mozilla.com/D53740
--HG--
extra : moz-landing-system : lando
This is generally pretty straightforward, and rewrites nearly all calls. It
skips the ones that it can detect using frame script globals like
`sendAsyncMessage`, though.
Differential Revision: https://phabricator.services.mozilla.com/D53740
--HG--
extra : moz-landing-system : lando
This is generally pretty straightforward, and rewrites nearly all calls. It
skips the ones that it can detect using frame script globals like
`sendAsyncMessage`, though.
Differential Revision: https://phabricator.services.mozilla.com/D53740
--HG--
extra : moz-landing-system : lando
The inclusions were removed with the following very crude script and the
resulting breakage was fixed up by hand. The manual fixups did either
revert the changes done by the script, replace a generic header with a more
specific one or replace a header with a forward declaration.
find . -name "*.idl" | grep -v web-platform | grep -v third_party | while read path; do
interfaces=$(grep "^\(class\|interface\).*:.*" "$path" | cut -d' ' -f2)
if [ -n "$interfaces" ]; then
if [[ "$interfaces" == *$'\n'* ]]; then
regexp="\("
for i in $interfaces; do regexp="$regexp$i\|"; done
regexp="${regexp%%\\\|}\)"
else
regexp="$interfaces"
fi
interface=$(basename "$path")
rg -l "#include.*${interface%%.idl}.h" . | while read path2; do
hits=$(grep -v "#include.*${interface%%.idl}.h" "$path2" | grep -c "$regexp" )
if [ $hits -eq 0 ]; then
echo "Removing ${interface} from ${path2}"
grep -v "#include.*${interface%%.idl}.h" "$path2" > "$path2".tmp
mv -f "$path2".tmp "$path2"
fi
done
fi
done
Differential Revision: https://phabricator.services.mozilla.com/D55443
--HG--
extra : moz-landing-system : lando
Initially this was going to be a simple cleanup: Remove some useless namespaces
here and there and so on, remove `using` statements from the header and so on.
But unfortunately, DOMIntersectionObserver.h (which is included in Element.h,
unnecessarily) ended up exposing `Element` unnamespaced to a lot of code, so I
had to fix that.
Differential Revision: https://phabricator.services.mozilla.com/D55316
--HG--
extra : moz-landing-system : lando
Starting from bug 1597679, `HTMLTextAreaElement::GetValueInternal()` stops
converting `\r\n` and `\r` with `\n` because it may be in hot path and the scan
cost of `\r` in `nsContentUtils::PlatformToDOMLineBreaks()` is redundant in
most cases.
However, there are still some paths to input `\r`. This patch makes them
call `nsContentUtils::PlatformToDOMLineBreaks()` if they have not done it.
Differential Revision: https://phabricator.services.mozilla.com/D54906
--HG--
extra : moz-landing-system : lando
The initialization cost of `AutoStyleCacheArray` is still expensive and it's
used only by `HTMLEditor`. Therefore, we should make it `Maybe` and construct
it only when the editor is an `HTMLEditor`.
Depends on D54253
Differential Revision: https://phabricator.services.mozilla.com/D54254
--HG--
extra : moz-landing-system : lando
Calling `AppendElement()` a lot causes the constructor appearing in profile.
`AutoTArray` has a constructor taking initialization list. Let's use it.
Differential Revision: https://phabricator.services.mozilla.com/D54253
--HG--
extra : moz-landing-system : lando
Sub classes of `nsITextControlElement` are only `HTMLInputElement` and
`HTMLTextAreaElement`. And both base class is
`nsGenericHTMLFormElementWithState`. Therefore, we can make
`nsITextControlElement` inherit `nsGenericHTMLFormElementWithState` and
make `HTMLInputElement` and `HTMLTextAreaElement` inherit
`nsITextControlElement`. Then, we can get rid of a lot of QI between
`nsINode`/`nsIContent`/`Element` and `nsITextControlElement` (and note that
some of them in a hot path).
Additionally, this patch renames `nsITextControlElement` to
`mozilla::TextControlElement`.
Differential Revision: https://phabricator.services.mozilla.com/D54330
--HG--
rename : dom/html/nsITextControlElement.h => dom/html/TextControlElement.h
extra : moz-landing-system : lando
Sub classes of `nsITextControlElement` are only `HTMLInputElement` and
`HTMLTextAreaElement`. And both base class is
`nsGenericHTMLFormElementWithState`. Therefore, we can make
`nsITextControlElement` inherit `nsGenericHTMLFormElementWithState` and
make `HTMLInputElement` and `HTMLTextAreaElement` inherit
`nsITextControlElement`. Then, we can get rid of a lot of QI between
`nsINode`/`nsIContent`/`Element` and `nsITextControlElement` (and note that
some of them in a hot path).
Additionally, this patch renames `nsITextControlElement` to
`mozilla::TextControlElement`.
Differential Revision: https://phabricator.services.mozilla.com/D54330
--HG--
rename : dom/html/nsITextControlElement.h => dom/html/TextControlElement.h
extra : moz-landing-system : lando
The initialization cost of `AutoStyleCacheArray` is still expensive and it's
used only by `HTMLEditor`. Therefore, we should make it `Maybe` and construct
it only when the editor is an `HTMLEditor`.
Depends on D54253
Differential Revision: https://phabricator.services.mozilla.com/D54254
--HG--
extra : moz-landing-system : lando
Calling `AppendElement()` a lot causes the constructor appearing in profile.
`AutoTArray` has a constructor taking initialization list. Let's use it.
Differential Revision: https://phabricator.services.mozilla.com/D54253
--HG--
extra : moz-landing-system : lando
This is a regression by bug 1568996. Although editor uses nsFrameSelection to
move caret, if frame is dirty, nsFrameSelection returns error.
So by bug 1568996, we flush layout before calling nsFrameSelection. But we
should stop flushing layout when we don't use nsFrameSelection.
Differential Revision: https://phabricator.services.mozilla.com/D53919
--HG--
extra : moz-landing-system : lando
After removing all mutation event listeners, the corresponding method
still returns `true`, so the new name is more accurate.
Differential Revision: https://phabricator.services.mozilla.com/D52342
--HG--
extra : moz-landing-system : lando
With making it take `WidgetKeyboardEvent*`, it won't need to return "handled"
state. However, when we implement `beforeinput` event, it needs to return
"canceled" state. Therefore, it should return `EditActionResult`.
Differential Revision: https://phabricator.services.mozilla.com/D51953
--HG--
extra : moz-landing-system : lando
Currently, "input" event is fired when the `AutoScriptBlocker` in `SetValue()`
is deleted. So, for keeping same behavior, the post processing after calling
`TextEditor` methods should be done before editor dispatches "input" event.
Fortunately, `TextInputListener::OnEditActionHandled()` is a good chance to
do that. Therefore, this patch makes it notify `TextControlState` and
`AutoTextControlHandlingState`.
Note that ideally, each method of `TextEditor` should return
`NS_ERROR_OUT_OF_MEMORY` coming from
`AutoTextControlHandlingState::OnEditActionHandled()`. However, it requires
a lot of changes in editor classes, and the case is really rare since editor
does not use fallible allocation. Therefore, it must be okay to crash in
editor if `OnEditActionHandled()` returns `NS_ERROR_OUT_OF_MEMORY`.
Depends on D51395
Differential Revision: https://phabricator.services.mozilla.com/D51396
--HG--
extra : moz-landing-system : lando
When closing window / documnet, if content modifies an element that has
contenteditable via unload event, it hits assertion in
`nsDocShell::EnsureEditorData` due to destroying document. So we should
return error before getting editing session.
Differential Revision: https://phabricator.services.mozilla.com/D50573
--HG--
extra : moz-landing-system : lando
This seems to go back to our initial contenteditable implementation, and I don't
think there's a reason <label> should work differently from anything else.
Differential Revision: https://phabricator.services.mozilla.com/D46929
--HG--
extra : moz-landing-system : lando
`uint32_t`, because `nsRange::ComparePoints` requires it -- by webidl
interface -- to be unsigned long.
Moreover it makes `RangeBoundaryBase`'s interface cleaner, because it
already exposes the offset as a `uint32_t`.
Differential Revision: https://phabricator.services.mozilla.com/D50054
--HG--
extra : moz-landing-system : lando
Most of these tests have been disabled for a long time; they run well
in the current test environment.
I intend to enable still more mochitests in a future patch.
Differential Revision: https://phabricator.services.mozilla.com/D49524
--HG--
extra : moz-landing-system : lando
* Make about:networking work on mobile
* I used the aboutMemory.css file as a guide for doing this
* The mobile CSS does not include the common.css, since that doesn't seem to work. But at least now the interfaces is functional. We should make it look nice in a follow-up.
Differential Revision: https://phabricator.services.mozilla.com/D49582
--HG--
extra : moz-landing-system : lando
When inserting `<li>` elements into `<ul>`, `<ol>` or `<li>` element,
`HTMLEditor::DoInsertHTMLWithContext()` removes unnecessary empty `<li>`
elements at insertion point. At this time, we've computed next insertion
point with removed `<li>` element. Therefore, insertion point goes out
from the DOM tree. This patch makes it compute new insertion point before
removing each empty `<li>` element.
Additionally, this patch adds some WPT data for testing this case. I verified
that Chrome passes the new tests too.
Differential Revision: https://phabricator.services.mozilla.com/D49394
--HG--
extra : moz-landing-system : lando
Most of these tests have been disabled for a long time; they run well
in the current test environment.
With the additional tests running, task times increase; I have added one
more test chunk for android mochitest-plain.
These tests were identified from a random sampling of mochitest manifests;
I intend to enable more mochitests in future patches.
Differential Revision: https://phabricator.services.mozilla.com/D48912
--HG--
extra : moz-landing-system : lando
Finally, `Document.execCommand()` still does not work fine if selection
starts from very start of block and/or end at very end of block because
`PromoteInlineRange()` extends selection range to contain the
containers, then, `SubtreeContentIterator` won't list up text nodes.
In this case, `RemoveInlinePropertyInternal()` expects that
`RemoveStyleInside()` removes text node style with creating
`<span>` elements. However, `RemoveStyleInsilde()` only handles
`Element`s and it handles elements from most-descendants.
Therefore, it cannot distinguish whether text node style comes
from removing inline elements or parent block.
This patch makes `RemoveInlinePropertyInternal()` collect
descendant text nodes in the range after handling all nodes in
the range except descendant text nodes, then, check the
final style of descendant text nodes, finally, remove the style
if coming from parent block.
Differential Revision: https://phabricator.services.mozilla.com/D47865
--HG--
extra : moz-landing-system : lando
If selection range is not in **one** text node, `RemoveInlinePropertyInternal()`
collects target nodes with `SubtreeContentIterator`. It only collects topmost
nodes which are **entirely** contained in the range (it's enough because their
descendants will be handled by `RemoveStyleInside()` recursively).
The reasons why it uses `SubtreeContentIterator` rather than
`PreContentIterator` must be:
1. Performance reason.
2. Assuming there are no multiple text nodes.
3. Not expects that user removes text node styles come from parent block.
The reason 2 is wrong because when removing a style, all browsers don't
join text nodes which was in removing element with adjacent text nodes.
(I.e., we cannot change this behavior for compatibility.)
The reason 3 is of course wrong we're struggling with this scenario.
Therefore, `RemoveInlinePropertyInternal()` needs to collect partially
selected text nodes by itself (if there are). Then, we can merge the
single text node selected case with the `for` loop.
Differential Revision: https://phabricator.services.mozilla.com/D47864
--HG--
extra : moz-landing-system : lando
For compatibility with Chrome, when removing inline style at block parent,
we should reset the style with creating `<span>` element whose `style`
attribute removes the style. We do this only in CSS mode, but we should do
it in HTML mode too.
This patch also makes `FontFaceStateCommand::SetState()` ignore `tt` value
if its root caller is `Document::ExecCommand()`. It was implemented for
composer to handle XUL command in bug 115922. Therefore, we should not do
this special handling on the web. If it were possible to separate this
change to another bug, it'd be nicer. But without this change, we'll have
a lot of regressions of `Document.execCommand("fontname")`. Therefore,
this is also fixed in this patch.
Note that this removes first `.ini` file selection because
the tests cannot be run without test number range parameter.
So, the sections are not used anymore.
Differential Revision: https://phabricator.services.mozilla.com/D47862
--HG--
extra : moz-landing-system : lando
For compatibility with Chrome, when removing inline style at block parent,
we should reset the style with creating `<span>` element whose `style`
attribute removes the style. We do this only in CSS mode, but we should do
it in HTML mode too.
This patch also makes `FontFaceStateCommand::SetState()` ignore `tt` value
if its root caller is `Document::ExecCommand()`. It was implemented for
composer to handle XUL command in bug 115922. Therefore, we should not do
this special handling on the web. If it were possible to separate this
change to another bug, it'd be nicer. But without this change, we'll have
a lot of regressions of `Document.execCommand("fontname")`. Therefore,
this is also fixed in this patch.
Note that this removes first `.ini` file selection because
the tests cannot be run without test number range parameter.
So, the sections are not used anymore.
Differential Revision: https://phabricator.services.mozilla.com/D47862
--HG--
extra : moz-landing-system : lando
Surprisingly, its `aChildOnly` is never set to `true` and if it were set to
`true`, it does unnecessary recursive calls. Therefore, we can make it
simpler.
Differential Revision: https://phabricator.services.mozilla.com/D47860
--HG--
extra : moz-landing-system : lando
This is related to bug 1530649. When using <span> element with contentedtiable,
we won't insert <br> element at last. When Korean IME on macOS commits
composition by space key, composition string has space.
Gekco removes U+0020 space when it is last character into editing host. To keep
whitespace, we have to replace with NBSP when it is last.
Differential Revision: https://phabricator.services.mozilla.com/D48146
--HG--
extra : moz-landing-system : lando
Both method take a DOM point with `nsCOMPtr<nsINode>*` and `int32_t*`.
This makes each caller complicated. Instead, we should use stack only class
to return both `EditorDOMPoint` and `nsresult`. I name it `EditResult`.
Additionally, this fixes a bug of `HTMLeditor::SplitStyleAboveRange()`. That
is not tracking new selection start point while it splits elements at end
of given range. This is detected by the debug assertion in
`ToRawRangeBoundary()` (i.e., this fix is required to pass some tests).
Differential Revision: https://phabricator.services.mozilla.com/D47858
--HG--
extra : moz-landing-system : lando
Most of these tests have been disabled for a long time; they run well
in the current test environment.
Differential Revision: https://phabricator.services.mozilla.com/D47616
--HG--
extra : moz-landing-system : lando
Instead, rely on the proper initialization of `mWrapColumn` via
`nsPlainTextSerializer::Init`.
This will allow to move `mWrapColumn` to `nsPlainTextSerializer::Settings::mWrapColumn`,
simplifying reasoning about it.
Differential Revision: https://phabricator.services.mozilla.com/D46129
--HG--
extra : moz-landing-system : lando
Currently, we can use chrome process's shortcut key with
`EventUtils.synthesizeKey()` with enabling `"test.events.async.enabled"` pref.
So, we should reconvert it to a mochitest for making it more stable.
Oddly, when I try to run this test as test-verify on macOS, it permanently
fails rendering resizer of `<textarea>` elements immediately after creation.
Therefore, this patch disables this test in test-verify on macOS.
Differential Revision: https://phabricator.services.mozilla.com/D46578
--HG--
rename : editor/libeditor/tests/browser_bug629172.js => editor/libeditor/tests/test_bug629172.html
extra : moz-landing-system : lando
Only for solving the order of `AutoEditInitRulesTrigger` destruction when
`HTMLEditor::Init()` calls `TextEditor::Init()`, `TextEditor` has unnecessary
counter and the initialization code is made harder to understand.
Therefore, this patch makes `TextEditor::Init()` and `HTMLEditor::Init()`
directly call `InitEditorContentAndSelection()` at appropriate time.
Additionally, `HTMLEditor::Init()` can call `EditorBase::Init()` instead of
`TextEditor::Init()` since `TextEditor::Init()` does nothing for `HTMLEditor`
actually.
Differential Revision: https://phabricator.services.mozilla.com/D45793
--HG--
extra : moz-landing-system : lando
Now, we can get rid of `TextEditRules` and `HTMLEditRules` completely.
And also this patch renames their cpp files to `TextEditSubActionHandler`
and `HTMLEditSubActionHandler`.
`TextEditor::Init()` and `HTMLEditor::Init()` are still complicated due to
`AutoEditInitRulesTrigger`. The following patch will remove it.
Differential Revision: https://phabricator.services.mozilla.com/D45792
--HG--
rename : editor/libeditor/HTMLEditRules.cpp => editor/libeditor/HTMLEditSubActionHandler.cpp
rename : editor/libeditor/TextEditRules.cpp => editor/libeditor/TextEditSubActionHandler.cpp
extra : moz-landing-system : lando
It takes a lot of `bool` out arguments. Therefore, we should make it a
stack only class and caller should retrieve only necessary information.
Differential Revision: https://phabricator.services.mozilla.com/D45787
--HG--
extra : moz-landing-system : lando
And also this patch make each `AutoEditSubActionNotifier` creator check
the result of `OnStartToHandleTopLevelEditSubAction()` at least for
`NS_ERROR_EDITOR_DESTROYED`.
We need to take care of its destructor's result later, though.
Differential Revision: https://phabricator.services.mozilla.com/D45786
--HG--
extra : moz-landing-system : lando
This is regression by bug 1530649.
After landing bug 1530649, we try to scan end point of replacement text. But
in this bug's situation, afterRun becomes same as current ws run by landing
bug 1530649. To get white space type of next of replacement end, we have to
scan around end point again.
Differential Revision: https://phabricator.services.mozilla.com/D45947
--HG--
extra : moz-landing-system : lando
`TextEditRules::DocumentIsEmpty()` is a wrapper of `TextEditor::IsEmpty()` so
that we can get rid of it simply.
`HTMLEditRules::DocumentIsEmpty()` needs to change. It's oddly checks only
`EditorBase::mPaddingBRElementForEmptyEditor` is `nullptr` or not. However,
the editor may be completely empty. And the result may be different from
`TextEditor::IsEmpty()` which is not overridden by `HTMLEditor`. For partially
solving this issue, this patch makes `HTMLEditor` overrides `IsEmpty()` and
optimizes `TextEditor::IsEmpty()`.
With this change, the caller of `HTMLEditRules::DocumentIsEmpty()` may behave
differently in the only caller, `HTMLEditor::SelectEntireDocument()`. And
unfortunately, its root called from `SelectAllCommand::DoCommand()`. However,
it does just collapse `Selection` into the root element (`<body>` or
`Document.documentElement`) if it returns `true`. Therefore, this change
must be safe since anyway `SelectionRefPtr()->SelectAllChildren()` with
the root element is exactly same as `SelectionRefPtr()->Collapse()` with
the empty root element if it's truly empty.
Differential Revision: https://phabricator.services.mozilla.com/D45784
--HG--
extra : moz-landing-system : lando
The main purpose of them is modifying
`TopLevelEditSubActionData::mChangedRange`. Therefore, they should be in the
struct.
Differential Revision: https://phabricator.services.mozilla.com/D45783
--HG--
extra : moz-landing-system : lando
It requires different preparation in `TextEditor` and `HTMLEditor` but it's
not split. Therefore, we should make it virtual and override it to use
different preparation code. Fortunately, its code is enough simple to
duplicate.
Additionally, this removes unnecessary code from `TextEditRules` and
`HTMLEditRules` including `WillDoAction()` and `DidDoAction()`!
Differential Revision: https://phabricator.services.mozilla.com/D45495
--HG--
extra : moz-landing-system : lando
Unfortunately, `EditSubAction::eInsertElement` is used by 4 methods and one
of them is too big method. But we should move what old `WillInsert()` does
into them for stop making anybody use `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D45494
--HG--
extra : moz-landing-system : lando
It does 4 different things so that it looks like a black-box from the
callers.
First, only `HTMLEditRules::WillDoAction()` refers `aCancel` out argument.
Therefore, it should check whether it's cancelled or not directly.
Next, `EnsureNoPaddingBRElementForEmptyEditor()` should be called by each
caller directly.
Then, the renaming part can be split to 2 methods. One is adjusting
caret position and the other preparing inline style for new content.
Unfortunately, this patch makes each caller messy. I think that for the
3rd job (i.e., adjusting caret position), each caller should retrieve the
adjusted caret position and use it directly instead of handling with
`Selection` in the future.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
It does 4 different things so that it looks like a black-box from the
callers.
First, only `HTMLEditRules::WillDoAction()` refers `aCancel` out argument.
Therefore, it should check whether it's cancelled or not directly.
Next, `EnsureNoPaddingBRElementForEmptyEditor()` should be called by each
caller directly.
Then, the renaming part can be split to 2 methods. One is adjusting
caret position and the other preparing inline style for new content.
Unfortunately, this patch makes each caller messy. I think that for the
3rd job (i.e., adjusting caret position), each caller should retrieve the
adjusted caret position and use it directly instead of handling with
`Selection` in the future.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
In `TextEditor::HandleDeleteSelection()`, we have only one path of returning
`EditActionIgnored()`. Therefore, it should call
`DeleteSelectionWithTransaction()` directly.
On the other hand, it's not clear in `HTMLEditor::HandleDeleteSelection()`
since it may be called recursively by its helper methods. Therefore,
this patch creates `HTMLEditor::HandleDeleteSelectionInternal()` for the
recursive calls and makes `HTMLEditor::HandleDeleteSelection()` call
`DeleteSelectionWithTransaction()` if nobody handled it and to the
`HTMLEditor` specific post-process in it.
Differential Revision: https://phabricator.services.mozilla.com/D45301
--HG--
extra : moz-landing-system : lando
`TextEditor::DeleteSelectionAsSubAction()` starts to handle all
"delete selection" sub-actions (i.e., even if the instance is `HTMLEditor`).
Therefore, `TextEditRules::WillDeleteSelection()` should be renamed to
`TextEditor::HandleDeleteSelection()` and we need to make it virtual.
Differential Revision: https://phabricator.services.mozilla.com/D45300
--HG--
extra : moz-landing-system : lando
Oddly, they are used only by `HTMLEditor`, but implemented by `TextEditRules`.
They cancels when the editor is in plaintext mode. So, actual things are
implemented by each caller. This patch cleans them up too.
Differential Revision: https://phabricator.services.mozilla.com/D45299
--HG--
extra : moz-landing-system : lando
And also this patch moves `TextEditRules::HandleNewLines()` and
`TextEditRules::DontEchoPassword()` to `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D45298
--HG--
extra : moz-landing-system : lando
There are only 3 callers and it does simple but different 2 things. One of
the callers is `HTMLEditRules::DidDeleteSelection()` so that if same things
are done by `TextEditor::DeleteSelectionAsSubAction()`, it does not need to
duplicate the code. Therefore, we need to duplicate the code into
`TextEditor::DeleteSelectionAsSubAction()` and `TextEditRules::WillSetText()`.
Then, `TextEditRules::WillSetText()` can avoid accessing `Selection` since
it still grabs the modified text node.
Note that only when it's called by `TextEditRules::DidDoAction()`,
`AutoTransactionsConserveSelection` has been set. However, neither
`DeleteNodeWithTransaction()` nor `DeleteNodeTransaction::DoTransaction()`
changes `Selection`. Therefore, it hasn't do anything. So, we can remove
it right now.
Differential Revision: https://phabricator.services.mozilla.com/D45294
--HG--
extra : moz-landing-system : lando
Only caller of it is `WillRemoveAbsolutePosition()` and it always sets
`*aHandled` to true before calling it. Therefore, it does not need to take
it as an argument.
Differential Revision: https://phabricator.services.mozilla.com/D45291
--HG--
extra : moz-landing-system : lando
Masayuki Nakano
Simon Giesecke
shindli
Emilio Cobos Álvarez
Boris Zbarsky
Mark Banner
Mihai Alexandru Michis
Sylvestre Ledru
Zibi Braniecki
Razvan Maries
Mirko Brodesser
Tetsuharu OHZEKI
Andreea Pavel
Jonathan Kingston
Makoto Kato
Emma Malysz
Chris Peterson
Csoregi Natalia
Kris Maglione
Noemi Erli
Daniel Varga
Gabriele Svelto
Julian Descottes
jeffin143
Coroiu Cristina
Cosmin Sabou
Hiroyuki Ikezoe
Sebastian Hengst
Edgar Chen
Alex Henrie
Mats Palmgren
Geoff Brown
Valentin Gosu
Tim Nguyen
Alexander Surkov
Christoph Kerschbaumer
Jorg K
Gurzau Raul