This is a best effort attempt at ensuring that the adverse impact of
reformatting the entire tree over the comments would be minimal. I've used a
combination of strategies including disabling of formatting, some manual
formatting and some changes to formatting to work around some clang-format
limitations.
Differential Revision: https://phabricator.services.mozilla.com/D13073
--HG--
extra : moz-landing-system : lando
This is a best effort attempt at ensuring that the adverse impact of
reformatting the entire tree over the comments would be minimal. I've used a
combination of strategies including disabling of formatting, some manual
formatting and some changes to formatting to work around some clang-format
limitations.
Differential Revision: https://phabricator.services.mozilla.com/D13073
--HG--
extra : moz-landing-system : lando
Android's test runner passes this test, so I would like to run
test_dom_input_event_on_htmleditor.html even if Android.
Differential Revision: https://phabricator.services.mozilla.com/D13040
--HG--
extra : moz-landing-system : lando
The usage of our specific "text" event is enough low (0.0003%). So, let's
stop dispatching the event in the default group of web content. Once we
release this new behavior, we can get rid of dispatching the event even in
chrome. Then, we can optimize the event order for new specs.
Differential Revision: https://phabricator.services.mozilla.com/D13034
--HG--
extra : moz-landing-system : lando
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.
The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:
* First, I don't think non-editable regions of an editable element should
be user-select: all.
* Second, it just doesn't work in Shadow DOM (the editable descendant count is
not kept up-to-date when not in the uncomposed doc), so nested
contenteditables behave differently inside vs. outside a Shadow Tree.
* Third, I think it's user hostile to just entirely disable selection if you
have a contenteditable descendant as a child of a user-select: all thing.
WebKit behaves like this patch in the following test-case (though not Blink):
https://crisal.io/tmp/user-select-all-contenteditable-descendant.html
Edge doesn't seem to support user-select: all at all (no pun intended).
But we don't allow to select anything at all which looks wrong.
* Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
and not even notice...).
In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.
The other change is `all` not overriding everything else. So, something like:
<div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>
Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.
This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.
This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.
This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...
In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.
This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.
This also fixes issues with br-skipping like not being able to insert content in
the following test-case:
<div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>
If you start moving to the left from the second line, for example.
I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.
There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.
Differential Revision: https://phabricator.services.mozilla.com/D12687
--HG--
extra : moz-landing-system : lando
TextEditor instance is created per <input> element which has text editor and
<textarea> element. Therefore, we should keep TextEditor slim as far as
possible.
Currently, TextEditor class size is 400 bytes on Win64. So, we should keep
512 bytes border.
Differential Revision: https://phabricator.services.mozilla.com/D12404
--HG--
extra : moz-landing-system : lando
A lot of listeners are now notified with RefPtr for concrete classes.
Therefore, we can reduce size of arrays to listeners without damage for
the performance.
Differential Revision: https://phabricator.services.mozilla.com/D12403
--HG--
extra : moz-landing-system : lando
Similar to EditorBase::mSavedSel, we can move EditorBase::mRangeUpdater too
because of it's referred only when there is AutoEditActionDataSetter instance
so that it also does not need to be in the cycle collection.
And now, it can be marked as MOZ_STACK_CLASS and remove cycle collection
support.
Differential Revision: https://phabricator.services.mozilla.com/D12402
--HG--
extra : moz-landing-system : lando
EditorBase::mSavedSel is used only by EditorBase methods which are called only
by AutoSelectionRestorer. Additionally, AutoSelectionRestorer requires
AutoEditActionDataSetter instance. So, we don't need to keep create for
editor instance anymore. And also we don't need to keep it in the cycle
collection.
Note that SelectionState class is also used by PlaceholderTransaction.
Therefore, we cannot make it MOZ_STACK_CLASS.
Differential Revision: https://phabricator.services.mozilla.com/D12401
--HG--
extra : moz-landing-system : lando
EditorBase::mDirection is set and clear only when
EditorBase::AutoEditActionDataSetter::SetTopLevelEditSubAction(). So, the
direction is related to the top level edit sub action, and we can move it
into AutoEditActionDataSetter.
Note that except EditSubAction::eDeleteSelectedContent, the relation between
sub-action and direction is fixed so that this patch checks the relation with
MOZ_ASSERT. If we could replace EditSubAction::eDeleteSelectedContent with
information of direction, we'd remove the new member of
AutoEditActionDataSetter, though.
Differential Revision: https://phabricator.services.mozilla.com/D12400
--HG--
extra : moz-landing-system : lando
EditorBase::mTopLevelEditSubAction is set only by
EditorBase::OnStartToHandleTopLevelEditSubAction() and
EditorBase::OnEndToHandleTopLevelEditSubAction() and they are called only by
AutoTopLevelEditSubActionNotifier().
So, this is used only in stack when a public method of editor is called.
Therefore, we can move it into EditorBase::AutoEditActionDataSetter. Then,
we can reduce heap allocation for editor instances.
Differential Revision: https://phabricator.services.mozilla.com/D12399
--HG--
extra : moz-landing-system : lando
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.
The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:
* First, I don't think non-editable regions of an editable element should
be user-select: all.
* Second, it just doesn't work in Shadow DOM (the editable descendant count is
not kept up-to-date when not in the uncomposed doc), so nested
contenteditables behave differently inside vs. outside a Shadow Tree.
* Third, I think it's user hostile to just entirely disable selection if you
have a contenteditable descendant as a child of a user-select: all thing.
WebKit behaves like this patch in the following test-case (though not Blink):
https://crisal.io/tmp/user-select-all-contenteditable-descendant.html
Edge doesn't seem to support user-select: all at all (no pun intended).
But we don't allow to select anything at all which looks wrong.
* Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
and not even notice...).
In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.
The other change is `all` not overriding everything else. So, something like:
<div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>
Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.
This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.
This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.
This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...
In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.
This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.
This also fixes issues with br-skipping like not being able to insert content in
the following test-case:
<div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>
If you start moving to the left from the second line, for example.
I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.
There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.
Differential Revision: https://phabricator.services.mozilla.com/D12687
--HG--
extra : moz-landing-system : lando
Currently, calling nsITableEditor.insertTableCell() does not cause dispatching
"input" event since it does not create AutoPlaceholderBatch. Additionally,
different from InsertTableRowsWithTransaction() and
InsertTableColumnsWithTransaction(), it does not create
AutoTopLevelEditSubActionNotifier.
Because of those APIs should work similarly, we should make it creates
both auto class instances.
Differential Revision: https://phabricator.services.mozilla.com/D12248
--HG--
extra : moz-landing-system : lando
When all editor text is replaced while handling a user operation, editor
needs to dispatch "input" event. Therefore, in such case, i.e., EditAction
is eReplaceText, TextEditor::SetTextAsSubAction() needs to handle it instead
of TextEditRules::WillSetText().
Differential Revision: https://phabricator.services.mozilla.com/D12246
--HG--
extra : moz-landing-system : lando
Currently, a lot of code dispatch "input" event and some of them dispatch
"input" event with wrong interface and/or values. Therefore this patch
creates nsContentUtils::DispatchInputEvent() to make all of them dispatch
correct event.
Unfortunately, due to bug 1506439, we cannot set pointer to refcountable
classes of MOZ_CAN_RUN_SCRIPT method to nullptr. Therefore, this patch
creates temporary RefPtr<TextEditor> a lot even though it makes damage to
the performance if it's in a hot path.
This patch makes eEditorInput event dispatched with
InternalEditorInputEvent when "input" event should be dispatched with
dom::InputEvent. However, this patch uses WidgetEvent whose message is
eUnidentifiedEvent and setting WidgetEvent::mSpecifiedEventType to
nsGkAtoms::oninput when "input" event should be dispatched with
dom::Event because we need to keep that eEditorInput and
InternalEditorInputEvent are mapped each other.
Differential Revision: https://phabricator.services.mozilla.com/D12244
--HG--
extra : moz-landing-system : lando
It's difficult to create new test which checks "input" events caused by
all edit operations especially when text is inserted from our UI. Therefore,
this adds "input" event type checks into existing tests.
Additionally, this adds new test for MozEditableElement.setUserInput() whose
behavior needs to be fixed in this bug.
Currently, InputEvent interface should be used only on text controls or
contenteditable editor when dispatching "input" event.
https://w3c.github.io/input-events/#events-inputevents
You may feel odd to use different event interface for same "input" events.
However, other browsers also use InputEvent interface only in the cases. So,
we should follow them for now.
Differential Revision: https://phabricator.services.mozilla.com/D12243
--HG--
extra : moz-landing-system : lando
This is the first part of hiding the implementation of nsIJSID behind the
interface added in Part 1, such that we can substitute that implementation out.
I had to make a couple of changes to fix the errors caused by the new behaviour
in GenerateQI.
Differential Revision: https://phabricator.services.mozilla.com/D2279
The current test has an incorrect expectation for the first full spell-checking,
it actually happens when first-time focus moves to contenteditable element
(which trigger dictionary updating), not right after document loaded.
Differential Revision: https://phabricator.services.mozilla.com/D11463
--HG--
extra : moz-landing-system : lando
Even when execCommand("insertorderedlist") and
execCommand("insertunorderedlist") remove existing lists, we need to set
InputEvent.inputType value to "insertOrderedList" or "insertUnorderedList".
Fortunately, the XPCOM method is used only for handling
execCommand("insertorderedlist") and execCommand("insertunorderedlist") on
Firefox. Therefore, we should make it set EditAction to
EditAction::eRemoveOrderedListElement or EditAction::RemoveUnorderedListElement.
Note that comm-central uses this method directly and uses "cmd_removeList"
which causes calling the XPCOM method with empty string. However, input
events for them won't be exposed to the web. Therefore, it's okay to set
EditAction::eRemoveListElement for the other cases.
Differential Revision: https://phabricator.services.mozilla.com/D11439
--HG--
extra : moz-landing-system : lando
Both Chrome and Safari dispatches 2 sets of "beforeinput" and "input" events
when user drag and drop content in same editor. One is "deleteByDrag" and
the other is "insertFromDrop". We need to follow same behavior since
it should be able to cancel only "deleteByDrag" or "insertFromDrop" when
we implement "beforeinput" event.
HTMLEditor::InsertObject() may handle asynchronously. And we don't need to
do anything additionally for HTMLEditor. Therefore, TextEditor::OnDrop()
can delete selection before inserting dropped content by itself.
Therefore, this patch makes TextEditor::OnDrop() call
TextEditor::PrepareToInsertContent() and EditorBase::FireInputEvent() by
itself.
Unfortunately, it seems that we cannot write automated tests for this.
Even if using synthesizeMouse() of EventUtils, synthesizing mouse events
won't generate "dragover" nor "drop" events. Perhaps, like EventUtils.js,
we need to do something with drag service, but it means that we cannot
emulate drag and drop in an editor.
Differential Revision: https://phabricator.services.mozilla.com/D11285
--HG--
extra : moz-landing-system : lando
Only TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext()
removes selected content before pasting from clipboard or inserting dropped
content, and they do same things. Therefore, they can share the code with
a helper method.
Note that this makes each root caller won't return NS_ERROR_EDITOR_DESTROYED
since the destruction is expected by web app.
Differential Revision: https://phabricator.services.mozilla.com/D11284
--HG--
extra : moz-landing-system : lando
TextEditor::OnDrop() calls TextEditor::InsertFromDataTransfer() or
HTMLEditor::InsertFromDataTransfer() and they are called only by
TextEditor::OnDrop().
TextEditor::InsertFromDataTransfer() calls only TextEditor::InsertTextAt()
and TextEditor::InsertTextAt() calls only TextEditor::InsertTextAsSubAction() if
insertion point is nullptr. Therefore, if the callers sets nullptr, they
should call TextEditor::InsertTextAsSubAction() directly. Then, we can
make TextEditor::InsertTextAt() require non-nullptr insertion point.
HTMLEditor::InsertFromDataTransfer() calls HTMLEditor::InsertObject(),
HTMLEditor::DoInsertHTMLWithContext() or TextEditor::InsertTextAt().
HTMLEditor::InsertObject() calls HTMLEditor::DoInsertHTMLWithContext()
directly or via BlobReader (in this case, calls asynchronously).
Unfortunately both HTMLEditor::InsertObject() and
HTMLEditor::DoInsertHTMLWithContext() are called by
HTMLEditor::InsertFromTransferable() which is paste event handler. Therefore,
we cannot make them require non-nullptr insertion point, though, anyway,
they cannot become simpler even if we could do that.
This patch marks them as MOZ_CAN_RUN_SCRIPT as far as possible and
makes them take |const EditorDOMPoint&| for insertion point.
Differential Revision: https://phabricator.services.mozilla.com/D11283
--HG--
extra : moz-landing-system : lando
When Selection is NOT collapsed, we remove selected content. Therefore,
web apps don't need to know range information of user operation. However, web
apps may want to know direction of the operation (backward or forward). E.g.,
web apps may just mark selected range as "deleted" and move caret before or
after the range.
Therefore, when computed EditAction is eDeleteWordBackward or
eDeleteToBeginningOfSoftLine, we should use eDeleteBackward instead. When it
is eDeleteWordForward or eDeleteToEndOfSoftLine, we should use eDeleteForward
instead.
Note that only on Windows, we follow behavior of richtext control (and Word).
That is, Ctrl + Backspace/Delete collapse from start of selected range to
start/end of current word. I.e., collapsing Selection to start first and
removing to start or end of current word is Windows's standard behavior.
Currently, we do this in DeleteSelectionAsSubAction() but every caller
specifies eNone to aDirection except DeleteSelectionAsAction(). So, we can
move this before re-computing EditAction in DeleteSelectionAsAction().
Differential Revision: https://phabricator.services.mozilla.com/D10992
--HG--
extra : moz-landing-system : lando
No one uses nsISpellChecker, so let's get rid of nsISpellChecker.
Depends on D10993
Differential Revision: https://phabricator.services.mozilla.com/D10994
--HG--
extra : moz-landing-system : lando
When creating an instance of nsISpellChecker, we always use
mozSpellChecker::Create. So we should use mozSpellChecker directly instead of
nsISpellChecker.
Differential Revision: https://phabricator.services.mozilla.com/D10993
--HG--
extra : moz-landing-system : lando
ResizerMouseMotionListener listens to "mousemove" events for resizers
or grabber to move absolutely position element and it calls only
HTMLEditor::MouseMove(). Fortunately, neither EditorEventListener not
HTMLEditorEventListener listens to "mousemove" events. Therefore, we
can use HTMLEditorEventListener instead.
Differential Revision: https://phabricator.services.mozilla.com/D10869
--HG--
extra : moz-landing-system : lando
DocumentResizeEventListener listens to only "resize" events of the window
and when it fired, it just calls HTMLEditor::RefreshResizers(). Fortunately,
neither EditorEventListener nor HTMLEditorEventListener listens to "resize"
events. Therefore, we can move this implementation into
HTMLEditorEventListener.
Differential Revision: https://phabricator.services.mozilla.com/D10866
--HG--
extra : moz-landing-system : lando
No one uses nsISpellChecker, so let's get rid of nsISpellChecker.
Differential Revision: https://phabricator.services.mozilla.com/D10994
--HG--
extra : rebase_source : 1caeba2edf7677258c0406f087eaa41e12cbb8a8
extra : histedit_source : 6a90572e8ecddf0f13fbe5b1abf1bedf1c84a4d9
When creating an instance of nsISpellChecker, we always use
mozSpellChecker::Create. So we should use mozSpellChecker directly instead of
nsISpellChecker.
Differential Revision: https://phabricator.services.mozilla.com/D10993
--HG--
extra : rebase_source : 0feffa60e6dc788904d212a77acbf416279af083
extra : histedit_source : b89cc8270c2df70ae414479f43ac30e8aa0a3d42
This fixes odd case of this API. GetSelectedElement() ignores non-element
nodes before an element node. This must be intended to allow Selection to
start from in an element node. However, current code allows to select starting
from previous text node. This patch changes this behavior to stop looking
for element node if non-element node appears before an element node.
Differential Revision: https://phabricator.services.mozilla.com/D10703
--HG--
extra : moz-landing-system : lando
The |for| loop in HTMLEditor::GetSelectedElement() does not allow to be
after an element node because if another element is found, returns nullptr,
and if another non-element node is found, lastElementInRange which is
result of the method is cleared. So, we checking lastElementInRange at
first of the for loop (i.e., immediately after calling
nsIContentIterator::Next()), we can get rid of use ugly bool flag.
Differential Revision: https://phabricator.services.mozilla.com/D10702
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetSelectedElement() uses |while| and calls
nsIContentIterator::Next() at the last line. Therefore, it cannot use
early-continue style. Because nsIContentIterator::Next() is always called,
it should be rewrite with |for|.
Differential Revision: https://phabricator.services.mozilla.com/D10701
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetSelectedElement() should use RangeBoundary to reduce auto
variables which are in large scope. So, first optimization block can be
optimized with RangeBoundary.
Then, the second block, for handling nsGkAtoms::href, does not need to
retrieve AnchorRef() nor FocusRef() since this is exactly same as
StartRef() and EndRef() of the first range when there is only one selected
range. So, the last |if| block in this block is not necessary since
this has already been handled by the first block.
Differential Revision: https://phabricator.services.mozilla.com/D10700
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetElementOrParentByTagNameInternal() may return <a> element
which does not match for nsGkAtoms::href or nsGkAtoms::anchor (in the former
case, it should return only an <a> element which has "href" attribute and
its value is not empty. in the latter case, it should return only <a> element
which has "name" attribute and its value is not empty).
This patch makes it won't check found element's name when nsGkAtoms::href or
nsGkAtoms::anchor is specified.
Differential Revision: https://phabricator.services.mozilla.com/D10699
--HG--
extra : moz-landing-system : lando
This takes back old behavior, 59 or earlier version.
|selectedElement| may be set to an element which is not what the caller is
looking for. Therefore, if the first element node in the selected range is
not what the caller is looking for, it should return nullptr.
This regression is caused by this changeset:
https://hg.mozilla.org/mozilla-central/rev/93c1d149d757 (bug 1432944)
Additionally, this patch modifies the automated test for conforming to
original idea of the API. This API must not be intended to retrieve
usual inline elements like <b>, <i>, etc.
Differential Revision: https://phabricator.services.mozilla.com/D10697
--HG--
extra : moz-landing-system : lando
DocumentResizeEventListener::HandleEvent() calls protected method,
HTMLEditor::RefreshResizersInternal(). However, this method is an event
handler, i.e., called when the editor is not handling an edit action.
Therefore, for ensuring AutoEditActionDataSetter instance, it should call
public method, nsIHTMLEditor::RefereshResizers() instead.
Differential Revision: https://phabricator.services.mozilla.com/D10706
--HG--
extra : moz-landing-system : lando
