As far as I've tested, Chrome does not throw exception even when editor is
destroyed or editor content is modified unexpectedly. So, we should return
`NS_OK` from most public methods of editor when internal methods return
`NS_ERROR_EDITOR_DESTROYED` or `NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE`.
Differential Revision: https://phabricator.services.mozilla.com/D20811
--HG--
extra : moz-landing-system : lando
We should stop handling splitting nodes if mutation event listeners move or
remove the split nodes unexpectedly because the post processors may not be
able to keep handling the nodes. For example, if a node is moved to outside
of editing host, we shouldn't touch it anymore due to non-editable.
This patch makes `EditorBase::DoSplitNode()` return new error for making
any parent callers stop their job, but note that the following patch makes
any public methods expose the new error as exception for compatibility with
Chrome.
Differential Revision: https://phabricator.services.mozilla.com/D20810
--HG--
extra : moz-landing-system : lando
Oddly, `SplitNodeTransaction::DoTransaction()` checks the result of
`EditorBase::DoSplitNode()` only when it's not allowed to change `Selection`.
We should make it always check the result.
Differential Revision: https://phabricator.services.mozilla.com/D20809
--HG--
extra : moz-landing-system : lando
`InsertTagCommand::DoCommandParams()` inserts given URL to `href` of `<a>` or
`src` of `<img>`. However, it treats the given URL includes only ASCII
characters. Therefore, we cannot insert URL including non-ASCII characters
with `execCommand("createLink")` nor `execCommand("insertImage")`.
This patch makes `nsHTMLDocument::ExecCommand()` set the param as `nsString`
and makes `InsertTagCommand::DoCommandParams()` retrieve it with `GetString()`.
Differential Revision: https://phabricator.services.mozilla.com/D20615
--HG--
extra : moz-landing-system : lando
`InsertTagCommand::DoCommandParams()` uses `Element::SetAttribute()` which takes
`nsAString` as attribute name. For avoiding unnecessary copy of attribute name,
we should make it use `Element::SetAttr()` which takes `nsAtom` instead.
Differential Revision: https://phabricator.services.mozilla.com/D20614
--HG--
extra : moz-landing-system : lando
"cmd_copyAndCollapseToEnd" is referred only by CopyPasteAssistent (even
including comm-central and BlueGriffon), but CopyPasteAssistent won't receive
"copy" command to send it.
So, it seems that we can get rid of a lot around CopyPasteAssistent but this
patch just changes the mapping in it ("copy" is mapped to "cmd_copy") and
removes command handlers of "cmd_copyAndCollapseToEnd" completely.
Differential Revision: https://phabricator.services.mozilla.com/D20612
--HG--
extra : moz-landing-system : lando
"cmd_setDocumentOptions" is never used now (even including comm-central and
BlueGriffon). So, we can stop supporting this command and we can get rid of
the command handler, SetDocumentOptionsCommand.
Differential Revision: https://phabricator.services.mozilla.com/D20611
--HG--
extra : moz-landing-system : lando
"cmd_clearUndo" is never used now (even including comm-central and BlueGriffon).
So, we can stop supporting this command and we can get rid of the command
handler, ClearUndoCommand.
Differential Revision: https://phabricator.services.mozilla.com/D20610
--HG--
extra : moz-landing-system : lando
Those probes are now expired and we got enough data:
- Almost no user uses the grip to move absolute positioned element
- There were over one thousand users using the inline table editor and the object resizers.
- Such users keep using even after we disabled the UIs by default.
Perhaps, such small number of users keep using the UIs, i.e., I guess the
number won't become smaller in short term. Therefore, this patch removes the
telemetry probes and members of HTMLEditor which are necessary to call
Telemetry API.
Differential Revision: https://phabricator.services.mozilla.com/D20609
--HG--
extra : moz-landing-system : lando
This code was already handling the world going away, but did not handle the case
of just getting unbound, which can happen if some selection listener (e.g.,
AccessibleCaret) flushes layout.
Differential Revision: https://phabricator.services.mozilla.com/D20469
--HG--
extra : moz-landing-system : lando
Only nsBlockFrame and its subclasses recognize the nsIFrame::eBlockFrame
flag, so we can replace the usage of the flag with either
nsIFrame::IsBlockFrameOrSubclass() or a do_QueryFrame().
Differential Revision: https://phabricator.services.mozilla.com/D20542
--HG--
extra : moz-landing-system : lando
Even less so on reframe, where it's just unsound to do so. I had to give a value
to eSetValue_Internal, since otherwise I cannot check for its presence. I can
further special-case the reframe case if you prefer.
Differential Revision: https://phabricator.services.mozilla.com/D20133
--HG--
extra : moz-landing-system : lando
DataTransfer should have more 2 constructors. One takes |const nsAString&| and
sets its data to the string. The other takes |nsITransferable*| and stores only
its data with DataTransferItem.
If given data is external resource like the case of `HTMLEditor::PasteTransferable()`,
we should copy its data to each DataTransferItem because nsITransferable is not
cycle-collectable, but DataTransfer may be grabbed by JS. Unfortunately, adding
new path to initialize DataTransfer with nsITransferable instance is too risky
because DataTransfer and DataTransferItem work together to initialize each of
them if DataTransfer is in external mode. Therefore, this patch makes the
new constructor temporarily sets it to in external mode, then, cache usable types
first, then, call `FillAllExternalData()` to make each DataTransferItem initializes
its data by itself, finally, make the constructor set it to internal mode and release
nsITransferable instance. This is ugly implementation but the most reasonable
way for now because:
- We don't need to change DataTransfer nor DataTransferItem a lot in this bug.
- We don't need to duplicate any code like a loop in `CacheExternalData()`.
In another bug, we should redesign DataTransfer and DataTransferItem to
make DataTransferable easier to be added new constructors.
Differential Revision: https://phabricator.services.mozilla.com/D19298
--HG--
extra : moz-landing-system : lando
InputEvent.dataTransfer should be set to non-null when InputEvent.inputType
is "insertFromPaste", "insertFromDrop" or "insertReplacementText" and
editor is an HTMLEditor instance:
https://rawgit.com/w3c/input-events/v1/index.html#dfn-datahttps://w3c.github.io/input-events/#dfn-data
("insertTranspose" and "insertFromYank" are not currently supported on Gecko.)
This patch makes nsContentUtils::DispatchInputEvent() take dataTransfer value
and EditorBase set it via AutoEditActionDataSetter like data value.
However, we need to create other constructors of DataTransfer to create its
read-only instances initialized with nsITransferable or nsAString. This will
be implemented by the following patch.
Differential Revision: https://phabricator.services.mozilla.com/D19297
--HG--
extra : moz-landing-system : lando
Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).
Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.
This patch makes given color value parsed and then serialized with same code
in style system. If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now. Additionally, when given value is invalid, sets the value
as-is for forward compatibility.
Note that automated tests will be added into input-events-exec-command.html
by the last patch.
1. https://github.com/w3c/input-events/issues/94#issuecomment-461061517
Differential Revision: https://phabricator.services.mozilla.com/D19295
--HG--
extra : moz-landing-system : lando
Although neither Chrome nor Safari does not set InputEvent.data value when
InputEvent.inputType is "fontName", but it's easy to implement. Therefore, this
patch implements it as declaration of Input Events.
This patch uses given value as-is. Perhaps, this shouldn't cause any problems
because such value can be set to Element.style.fontFamily without any changes.
Note that automated test will be added into WPT later.
Differential Revision: https://phabricator.services.mozilla.com/D19294
--HG--
extra : moz-landing-system : lando
Although neither Chrome nor Safari does not set InputEvent.data value when
InputEvent.inputType is "insertLink", but it's easy to implement. Therefore,
this patch implements it as declaration of Input Events.
This patch sets the value to raw href attribute value because we create
<a> element without absolute URI when web apps call execCommand("createLink")
with relative URI.
Differential Revision: https://phabricator.services.mozilla.com/D19291
--HG--
extra : moz-landing-system : lando
When InputEvent.inputType is "formatSetBlockTextDirection" or
"formatSetInlineTextDirection", InputEvent.data value should be one of
"ltr", "rtl", "auto" or "null".
https://rawgit.com/w3c/input-events/v1/index.html#dfn-datahttps://w3c.github.io/input-events/#dfn-data
We only supports "ltr" and "rtl" when user switches the direction with
Accel + Shift + X. Therefore this patch makes EditorBase set the data
to "ltr" or "rtl".
Oddly, with synthesizing the shortcut keys, the command is not executed
properly in the automated test. Therefore, this patch dispatches the
command directly.
Differential Revision: https://phabricator.services.mozilla.com/D19288
--HG--
extra : moz-landing-system : lando
https://rawgit.com/w3c/input-events/v1/index.html#dfn-datahttps://w3c.github.io/input-events/#dfn-data
Both Input Events Level 1 and Level 2 declare that InputEvent.data should be
set to inserting string only on TextEditor when InputEvent.inputType is
"insertFromPaste", "insertFromPasteAsQuotation", "insertFromDrop",
"insertTranspose", "insertReplacementText" or "insertFromYank".
Currently, we support only "insertFromPaste", "insertFromDrop",
"insertReplacementText". Therefore, this patch makes TextEditor set
EditorBase::mEditActionData::mData only for them (and the instance is not
HTMLEditor's).
Differential Revision: https://phabricator.services.mozilla.com/D19287
--HG--
extra : moz-landing-system : lando
There are few things that are either Fennec-specific or don't work
currently under GeckoView w/ e10s under TestRunnerActivity. Disable
these so we can get some testing going in automation.
Differential Revision: https://phabricator.services.mozilla.com/D19016
--HG--
extra : moz-landing-system : lando
No one uses CheckCurrentWordNoSuggest. So I would like to get a rid of this
method. And this method is only user of mozSpellChecker::CheckWord. So
mozSpellChecker::CheckWord shouldn't allow that aSuggestions is nullptr on
content process since we already have async API as mozSpellChecker::CheckWords.
Differential Revision: https://phabricator.services.mozilla.com/D19303
--HG--
extra : moz-landing-system : lando
Summary:
Editor changes caret visibility during drag and drop. But when destroying
editor, we don't restore caret state. So we should restore it when destroying
editor.
Tags: #secure-revision
Bug #: 1496118
Differential Revision: https://phabricator.services.mozilla.com/D18923
--HG--
extra : rebase_source : 157f2c9e69857c7f7adb916af2a319392c4d125a
***
Bug 1514594: Part 3a - Change ChromeUtils.import to return an exports object; not pollute global. r=mccr8
This changes the behavior of ChromeUtils.import() to return an exports object,
rather than a module global, in all cases except when `null` is passed as a
second argument, and changes the default behavior not to pollute the global
scope with the module's exports. Thus, the following code written for the old
model:
ChromeUtils.import("resource://gre/modules/Services.jsm");
is approximately the same as the following, in the new model:
var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
Since the two behaviors are mutually incompatible, this patch will land with a
scripted rewrite to update all existing callers to use the new model rather
than the old.
***
Bug 1514594: Part 3b - Mass rewrite all JS code to use the new ChromeUtils.import API. rs=Gijs
This was done using the followng script:
https://bitbucket.org/kmaglione/m-c-rewrites/src/tip/processors/cu-import-exports.jsm
***
Bug 1514594: Part 3c - Update ESLint plugin for ChromeUtils.import API changes. r=Standard8
Differential Revision: https://phabricator.services.mozilla.com/D16747
***
Bug 1514594: Part 3d - Remove/fix hundreds of duplicate imports from sync tests. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16748
***
Bug 1514594: Part 3e - Remove no-op ChromeUtils.import() calls. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16749
***
Bug 1514594: Part 3f.1 - Cleanup various test corner cases after mass rewrite. r=Gijs
***
Bug 1514594: Part 3f.2 - Cleanup various non-test corner cases after mass rewrite. r=Gijs
Differential Revision: https://phabricator.services.mozilla.com/D16750
--HG--
extra : rebase_source : 359574ee3064c90f33bf36c2ebe3159a24cc8895
extra : histedit_source : b93c8f42808b1599f9122d7842d2c0b3e656a594%2C64a3a4e3359dc889e2ab2b49461bab9e27fc10a7
GitLab's comment calls scrollTop on input event handler. The scollTop may cause
reflow. When causing reflow, editor is destroyed and initialized again. Then
nsTextEditorState will set current value to editor. But this is failure.
By bug 1465702, selection is cached in edit action. When initializing editor,
selection controller is updated, so selection into cache becomes invalid. It
means that all selection methods will return error since document is different.
So we should update selection cache when initializing editor.
Also, I cannot create test case for this situation since we have to cause reflow
in input and/or composition event. Do you have any idea?
Differential Revision: https://phabricator.services.mozilla.com/D16944
--HG--
extra : moz-landing-system : lando
For mozInlineSpellChecker, I would like to add promse-based method to check
word. But this method is
- On content process, it works by async.
- On chrome process, it works by sync.
Some reftests with non-e10s don't wait that spellchecker is finished correctly
due to race condition of spellchecker dictionary update. So it uses sync'd
promise on chrome.
Differential Revision: https://phabricator.services.mozilla.com/D14836
--HG--
extra : rebase_source : 63d8fb1b3a6f8754f5b7604ea920c1a26bcd4f5b
This patch makes ContentIteratorBase, PostContentIterator, PreContentIterator
and ContentSubtreeIterator classes non-refcountable because most users can
create their instances in stack and such users may be in a hot path. So,
we can save a lot of cost of instantiation.
Unfortunately, only ScriptableContentIterator creates one of the concrete
classes and needs to destroy it properly. Therefore, its
EnsureContentIterator(), destructor, traverse and unlink code becomes messy.
However, ScriptableContentIterator was designed for automated tests and we
need to maintain it not so many times. Therefore, improvement of other
users must be worthwhiler than this demerit.
Differential Revision: https://phabricator.services.mozilla.com/D15928
--HG--
extra : moz-landing-system : lando
nsFilteredContentIterator is used only by TextServicesDocument and there is
no reason that it should be derived from nsIContentIterator except consistency.
Additionally, it's now only class which is derived from nsIContentIterator
except ContentIteratorBase. So, after this change, we can get rid of
nsIContentIterator completely.
This patch moves nsFilteredContentIterator into mozilla namespace and
makes TextServicesDocument treat FilteredContentIterator directly instead of
nsIContentIterator interface.
Differential Revision: https://phabricator.services.mozilla.com/D15925
--HG--
rename : editor/spellchecker/nsFilteredContentIterator.cpp => editor/spellchecker/FilteredContentIterator.cpp
rename : editor/spellchecker/nsFilteredContentIterator.h => editor/spellchecker/FilteredContentIterator.h
extra : moz-landing-system : lando
Now, all users of PostContentIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15923
--HG--
extra : moz-landing-system : lando
Now, all users of PreContentIterator can access it directly. This patch makes
them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15922
--HG--
extra : moz-landing-system : lando
Now, all users of ContentSubtreeIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15920
--HG--
extra : moz-landing-system : lando
This patch makes ContentIteratorBase, PostContentIterator, PreContentIterator
and ContentSubtreeIterator classes non-refcountable because most users can
create their instances in stack and such users may be in a hot path. So,
we can save a lot of cost of instantiation.
Unfortunately, only ScriptableContentIterator creates one of the concrete
classes and needs to destroy it properly. Therefore, its
EnsureContentIterator(), destructor, traverse and unlink code becomes messy.
However, ScriptableContentIterator was designed for automated tests and we
need to maintain it not so many times. Therefore, improvement of other
users must be worthwhiler than this demerit.
Differential Revision: https://phabricator.services.mozilla.com/D15928
--HG--
extra : moz-landing-system : lando
nsFilteredContentIterator is used only by TextServicesDocument and there is
no reason that it should be derived from nsIContentIterator except consistency.
Additionally, it's now only class which is derived from nsIContentIterator
except ContentIteratorBase. So, after this change, we can get rid of
nsIContentIterator completely.
This patch moves nsFilteredContentIterator into mozilla namespace and
makes TextServicesDocument treat FilteredContentIterator directly instead of
nsIContentIterator interface.
Differential Revision: https://phabricator.services.mozilla.com/D15925
--HG--
rename : editor/spellchecker/nsFilteredContentIterator.cpp => editor/spellchecker/FilteredContentIterator.cpp
rename : editor/spellchecker/nsFilteredContentIterator.h => editor/spellchecker/FilteredContentIterator.h
extra : moz-landing-system : lando
Now, all users of PostContentIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15923
--HG--
extra : moz-landing-system : lando
Now, all users of PreContentIterator can access it directly. This patch makes
them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15922
--HG--
extra : moz-landing-system : lando
Now, all users of ContentSubtreeIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15920
--HG--
extra : moz-landing-system : lando
Editor initializes selection and input context (via IMEStateManager) on focus
event. But if content script calls stopImmediatePropagation on focus event,
editor cannot initialize these since editor cannot receive focus event.
It means that Android widget doesn't open virtual keyboard since
GeckoEditableSupport::SetInputContext isn't called. Also, Firefox desktop
doesn't show caret in this situation since selection isn't initialized in
editor.
So the event listener of focus and blur event should use system group.
Differential Revision: https://phabricator.services.mozilla.com/D15921
--HG--
extra : rebase_source : 71ab7e4aa043bac43e307dd6dfe4e84073d5154f
This patch implements InputType.inputType which is declared by Input Events.
The attribute has already been implemented by Chrome and Safari. Chrome
implements Input Events Level 1, but Safari implements Input Events Level 2.
Difference between them is only whether it supports "insertFromComposition",
"deleteByComposition" and "deleteCompositionText". This patch makes the
level switchable with pref and takes Level 1 by default because Level 2 is
still unstable around event order with composition events.
For reducing string copy cost at dispatching "input" event, this patch
makes EditorInternalInputEvent store valid input-type as enum class,
EditorInputType and resolves it to string value when
dom::InputEvent::GetInputType() is called. Note that the reason why
this patch names the enum class as EditorInputType is, there is InputType
enum class already for avoiding conflict the name, this appends "Editor"
prefix because "input" and "beforeinput" events are fired only when an
editor has focus.
Differential Revision: https://phabricator.services.mozilla.com/D14128
--HG--
extra : moz-landing-system : lando
