Summary:
The latest version of Gecko doesn't crash by this HTML, but I would like to add
this for the future.
Bug #: 1441619
Differential Revision: https://phabricator.services.mozilla.com/D14846
--HG--
extra : rebase_source : 5633e508d1c1f405465857a71f7c544fbb3782e8
- modify line wrap up to 80 chars; (tw=80)
- modify size of tab to 2 chars everywhere; (sts=2, sw=2)
--HG--
extra : rebase_source : 7eedce0311b340c9a5a1265dc42d3121cc0f32a0
extra : amend_source : 9cb4ffdd5005f5c4c14172390dd00b04b2066cd7
After fixing bug 1427060, HTMLEditor treats attribute of style as nullptr.
However, if empty string is used to call NS_Atomize(), it returns
nsGkAtoms::_empty. Therefore, HTMLEditor fails to check whether attribute is
specified or not with nullptr check since some root callers sets
nsGkAtoms::_empty instead of nullptr.
This patch makes HTMLEditor always use nullptr for empty string of attribute
of style with wrapping NS_Atomize() with AtomzieAttribute(). Additionally,
for safer change, this patch makes PropItem and TypeInState treat
nsGkAtom::_empty as nullptr.
Differential Revision: https://phabricator.services.mozilla.com/D13203
--HG--
extra : moz-landing-system : lando
Storage events are fired either directly after getting response from synchronous SetItem call or through observers. When a new onstorage event listener is added, we sycnhronously register an observer in the parent process. There's always only one observer actor per content process.
PBackgroundLSDatabase is now managed by a new PBackgroundLSObject protocol. PBackgroundLSObject is needed to eliminate the need to pass the principal info and document URI everytime a write operation occurs.
Preparation of an observer shares some states with preparation of a datastore, so common stuff now lives in LSRequestBase and preparation of a datastore now implements a nested state machine.
This patch was enhanced by asuth to drop observers only when the last storage listener is removed.
EventListenerRemoved is invoked on any removal, not just the final removal, so we need to make sure it's the final removal before dropping observer.
The implementation is based on a cache (datastore) living in the parent process and sync IPC calls initiated from content processes.
IPC communication is done using per principal/origin database actors which connect to the datastore.
The synchronous blocking of the main thread is done by creating a nested event target and spinning the event loop.
Turn all const lists and related attributes into cenums, to provide a
vague sense of type safety.
Depends on D11715
Differential Revision: https://phabricator.services.mozilla.com/D11716
--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
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
Makoto Kato
Bogdan Tara
Ehsan Akhgari
Edgar Chen
Benjamin Bouvier
Sylvestre Ledru
Razvan Maries
Masayuki Nakano
Jan Varga
Geoff Brown
Kyle Machulis
Cosmin Sabou
Andreea Pavel
Emilio Cobos Álvarez
Brindusan Cristian
Tom Schuster
Csoregi Natalia
Nika Layzell
Dorel Luca
Gurzau Raul