`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
Masayuki Nakano
Emilio Cobos Álvarez
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
shindli
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