The out params mean the last collapsed selection range's result (although,
the meaning is odd and offset and length are not overwritten when there is
another collapsed range and it causes `DeleteNodeTransaction`). Additionally,
when and only when `DeleteNodeTransaction` and `DeleteTextTransaction` are
added to the `EditAggregationTransaction` created by
`CreateTransactionForSelection()`. Therefore, same result can be looked for
from its only caller, `DeleteSelectionWithTransaction()`.
Note that this makes the method slower if there are too many selection ranges,
but such case must be rare because:
1. We can assume that users rarely use multiple selection ranges for removing
multiple ranges of content except table.
2. Multiple selection is supported only by Gecko. Therefore, web apps must
not use multiple selection for this purpose.
So, it must be okay to use this slower approach for making the methods simpler.
If it'd become damage to some benchmarks, let's create faster access to get
transaction type.
Depends on D72293
Differential Revision: https://phabricator.services.mozilla.com/D72294
`HTMLEditor::DeleteSelectionWithTransaction()` calls `EditorBase`'s overridden
method and handles `nsIEditor::eStrip` case. Therefore, we can rename it with
stop calling the `EditorBase::DeleteSelectionWithTransaction()`, and make it
called by `EditorBase::DeleteSelectionWithTransaction()` only when it's
necessary.
Additionally, we can make all internal method callers of editor classes always
set `nsIEditor::eNoStrip` if the instance is `TextEditor`. This must make
the code easier to understand.
Depends on D72292
Differential Revision: https://phabricator.services.mozilla.com/D72293
The parameter is used only by `EditorBase::CreateTxnForDeleteRange()` to
extend collapsed range, but it accepts only `nsIEditor::eNext` and
`nsIEditor::ePrevious`. Therefore, using `nsIEditor::EDirection` does not
make sense. Instead, they should use new `enum class`,
`HowToHandleCollapsedRange`.
Depends on D72291
Differential Revision: https://phabricator.services.mozilla.com/D72292
`DeleteSelection*()` are members of `TextEditor`, but they are also used by
`HTMLEditor`. Therefore, they and pref cache members for them should be
in `EditorBase` too.
Depends on D71911
Differential Revision: https://phabricator.services.mozilla.com/D72290
Currently, all input (including user pastes) to an input field is truncated to
`maxlength`. This diff disables truncation for user pastes.
When (1) `GetEditAction` is `ePaste`, `ePasteAsQuotation`, `eDrop`, or
`eReplaceText` (ie we are dealing with a paste) and (2) `GetEditActionPrincipal`
is `nullptr` (ie we are dealing with a user edit and not a JS edit), allow a
paste without truncation. That means that, in this case, we will return
`EditActionIgnored` instead of trying to truncate the string.
This behavior is controlled by a new preference `editor.truncate_user_pastes`
which is `false` by default (set in `StaticPrefList.yaml`).
We also modify `editor/libeditor/tests/test_bug603556.html` which currently
expects the output of a paste longer than maxlength to be truncated.
Testing: Created
`editor/libeditor/tests/test_pasting_text_longer_than_maxlength.html` which
checks if a user can paste a password longer than `maxlength` and if the field
is then marked as `tooLong` (this was the original concern of the reporter of
Bug 1320229), and
`editor/libeditor/tests/test_setting_value_longer_than_maxlength_with_setUserInput.html`
which checks if `eReplaceText` has consistent behavior regardless of whether the
field has an associated editor (this test works by calling `setUserInput()`
before and after the element gets focus.) `./mach test editor/libeditor` tests
pass.
Differential Revision: https://phabricator.services.mozilla.com/D71689
It's inherited only by `PlaceholderTransaction` and used only for QI.
Therefore, we can get rid of it.
Additionally, this makes storing `PlaceholderTransaction` and
`CompositionTransaction` in `PlaceholderTransaction` faster and safer with
`WeakPtr`.
Finally, this makes `PlaceholderTransaction` always have non-null name
because it caused a lot of useless warnings in
`EditAggregationTransaction::GetName()` and
`PlaceholderTransaction::GetTxnName()`.
Differential Revision: https://phabricator.services.mozilla.com/D71910
In a lot of places in libeditor, we do nothing if given transaction is not
our edit transaction classes' instance. Therefore, it's better to have
casting virtual method in `nsITransaction` for performance because QI cost
may not be cheap.
Differential Revision: https://phabricator.services.mozilla.com/D71908
Users have much better, easier alternatives, like
DOMWindowUtils.{loadSheetUsingURIString,removeSheet}, which we use to
replace the only caller that exists in mozilla-central (the editor
element, which loads EditorOverride.css).
This allows to clean up the style system and editor. There are other
callers in comm-central, but it seems they can switch to DOMWindowUtils
trivially, as the DOMWindowUtils APIs also use the system principal and
thus they can load any URL.
I'll make sure to give them some time with the migration and/or help
out of course.
Differential Revision: https://phabricator.services.mozilla.com/D71263
When `HTMLEditor` is notified of content changes, it may add a runnable method
`HTMLEditor::OnModifyDocument` or `HTMLEditor::NotifyRootChanged` for each
notification. However, their code do not need running twice nor more. This
could cause performance issues on complicated web apps which sets `innerHTML`
at every key press.
Differential Revision: https://phabricator.services.mozilla.com/D71001
This patch also names the former to `GetInclusiveAncestorBlockElement()` and
the latter to `GetAncestorBlockElement()` for consistency with modern DOM
API names.
Depends on D70882
Differential Revision: https://phabricator.services.mozilla.com/D70883
It's a virtual method which always returns true if `TextEditor`. Therefore,
we can move it into `HTMLEditUtils` and we can make the only caller of
`EditorBase` check `IsTextEditor()` instead.
Depends on D70880
Differential Revision: https://phabricator.services.mozilla.com/D70882
Actually, they are used only by `HTMLEditor` because `TextEditor` finally
returns `true` for any cases in `TextEditor`, but the users are overridden by
`HTMLEditor` and never used by `HTMLEditor`. Therefore, we cam move them
into `HTMLEditUtils`.
Depends on D70878
Differential Revision: https://phabricator.services.mozilla.com/D70879
--HG--
extra : moz-landing-system : lando
Their users should use `EditorDOMPoint` or something instead.
This patch cleans up `EditorBase::DoJoinNodes()` too because of their heavy
user. It requires only joined nodes because `aParent` is used only for
removing `aContentToJoin`, but we now have `nsINode::Remove()` which does
not require parent node and never fails.
Depends on D70877
Differential Revision: https://phabricator.services.mozilla.com/D70878
--HG--
extra : moz-landing-system : lando
A lot of editor code refers `nsINode::IsText()` directly so that we don't
need to keep the too simple editor API anymore.
Depends on D70876
Differential Revision: https://phabricator.services.mozilla.com/D70877
--HG--
extra : moz-landing-system : lando
It's only non-`HTMLEditor` user is `EditorBase::JoinNodesDeepWithTransaction()`,
but it's called only by `HTMLEditor`. Therefore, we can move it into
`HTMLEditUtils` and move `EditorBase::JoinNodesDeepWithTransaction()` to
`HTMLEditor`.
Depends on D70875
Differential Revision: https://phabricator.services.mozilla.com/D70876
--HG--
extra : moz-landing-system : lando
Due to the include hell, `EditorBase.h` cannot include `EditorUtils.h`.
Therefore we need these 3 methods once. Additionally, `IsModifiableNode()`
is really odd method and looks like that it's used for the following 2 purposes:
1. Simply can be editable.
2. Can be removed from parent.
For the former case, we should sort out it with
`EditorUtils::IsEditableContent()`, but for now, this patch moves it to
`HTMLEditUtils::IsSimplyEditable()`. On the other hand, for the latter case,
we obviously has a bug. Therefore, this patch creates
`HTMLEditUtils::IsRemovableFromParentNode()` and make it check whether the
removing node is also editable.
Unfortunately, `EditorUtils::IsEditableContent()` needs to take editor type.
But it's most callers are in `HTMLEditor` and most of remains are in
common methods of `EditorBase`. I guess we could remove this ugly argument
in the future.
Depends on D70874
Differential Revision: https://phabricator.services.mozilla.com/D70875
--HG--
extra : moz-landing-system : lando
`HTMLEditor::IsBlockNode()` is a virtual method derived from
`EditorBase::IsBlockNode()`. For the performance, `EditorBase`'s one just
return false for text node and `<br>` element. However, these check cost
is really cheap in these days. Therefore, we can make `TextEditor` also
use rich API.
This patch moves `HTMLEditor::NodeIsBlockStatic()` and
`HTMLEditor::NodeIsInlineStatic()` to `HTMLEditUtils`, and removes the wrapper
of the fommer, `HTMLEditor::IsBlockNode()` and `WSRunScanner::IsBlockNode()`.
Differential Revision: https://phabricator.services.mozilla.com/D70874
--HG--
extra : moz-landing-system : lando
We can relax about `GetElementOrParentElement*()` because they just return
`nullptr` when selection anchor is a `Document` node.
Additionally, this patch renames the internal APIs to the names similar to
modern DOM API.
Finally, this adds automated test for
`nsIHTMLEditor.getElementOrParentByTagName()`.
Differential Revision: https://phabricator.services.mozilla.com/D70152
--HG--
extra : moz-landing-system : lando
When it refers computed value of style, it may flush pending notifications.
Therefore, they should be marked as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D70151
--HG--
extra : moz-landing-system : lando
Some methods take `StyleType` to work them with specified values or computed
values. This method hides `StyleType` into `CSSEditUtils` and splits the
public methods which took `StyleType` to 2 methods, one is for working with
specified values, the other is for working with computed values.
Additionally, this patch fixes some argument name and type.
Differential Revision: https://phabricator.services.mozilla.com/D70149
--HG--
extra : moz-landing-system : lando
When it refers computed value of style, it may flush pending notifications.
Therefore, they should be marked as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D70151
--HG--
extra : moz-landing-system : lando
Some methods take `StyleType` to work them with specified values or computed
values. This method hides `StyleType` into `CSSEditUtils` and splits the
public methods which took `StyleType` to 2 methods, one is for working with
specified values, the other is for working with computed values.
Additionally, this patch fixes some argument name and type.
Differential Revision: https://phabricator.services.mozilla.com/D70149
--HG--
extra : moz-landing-system : lando
Since the test goes through all moz.build files disregarding DIRS and
the conditions that may disable directories, in some cases, moz.builds
can fail to be evaluated properly because of missing variables in
config.status. This time (because it's not the first), it's
LLVM_DLLTOOL.
After fixing that, it turns out many of the files/directories pointed to
by Files() directives were removed or moved.
While here, make the test script python3-ready.
Differential Revision: https://phabricator.services.mozilla.com/D70157
--HG--
extra : moz-landing-system : lando
underline and strike use `text-decoration` property, which is a
shorthand and may include other longhand property values, e.g.
`text-decoration-color`. In order to set `aAll` flag correctly, we
should not just compare the computed values between `firstValue` and
`theValue`. This patch makes the HTMLStyleEditor more independent of
the serializations of the computed values on text-decoration.
If https://github.com/w3c/editing/issues/241 is accepted, we can just
replace `text-decoration` with `text-decoration-line` and drop this
patch.
Differential Revision: https://phabricator.services.mozilla.com/D69619
--HG--
extra : moz-landing-system : lando
Only `TypeInState` stores last caret position. So, only it can detect the
case caret is moved from outside of `<a href>` element and is now at start
or end of it.
Note that this patch does not assume `<a href>` has an empty text node with
another inline element. If we supported it, the loop would be more complicated
and it's really unrealistic edge case. Therefore, it's reasonable to ignore
the case.
And also this patch works with `ArrowUp`/`ArrowDown`/`PageUp`/`PageDown`
cases. However, I have no idea to reject such cases. But I guess that
it does not so bad behavior for users because caret does not moved in
a text node in `<a href>`.
Depends on D69479
Differential Revision: https://phabricator.services.mozilla.com/D69480
--HG--
extra : moz-landing-system : lando
The selection validation should be checked in the constructor of
`ParagraphStateAtSelection` because
`EditorBase::IsSelectionRangeContainerNotContent()` needs edit action data.
Additionally, this patch adds a test for `nsIHTMLEditor.getParagraphState()`.
Differential Revision: https://phabricator.services.mozilla.com/D69316
--HG--
extra : moz-landing-system : lando
I'm still not sure how the crash occurs especially on Thunderbird. However,
it could be possible if `pointToInsert` is modified with an orphan node (i.e.,
when `pointToInsert.GetContainer()` returns `nullptr`). Therefore, this patch
makes it check whether the inserted node stays at expected position or not,
and if it's not, make it keep inserting next content nodes to previous position
because it must look like odd that inserting to different position.
Differential Revision: https://phabricator.services.mozilla.com/D69154
--HG--
extra : moz-landing-system : lando
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's __attribute__((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Differential Revision: https://phabricator.services.mozilla.com/D69315
--HG--
extra : moz-landing-system : lando
`AlignStateAtSelection` class is instantiated outside of editor class so that
we shouldn't make each user guarantee that there is selection range
(fortunately, the putting off cost is really low).
And as far as I tested, Blink and WebKit does not throw exception when
`Document.qeuryCommand*("justify*")` is called without selection ranges.
So, this patch also prevents exception in this situation.
Differential Revision: https://phabricator.services.mozilla.com/D68755
--HG--
extra : moz-landing-system : lando
Now, `WSType` is used only by `WSRunScanner`, `WSRunObject` and `WSScanResult`.
We should hide this mysterious `enum` from other classes for making other
developers easier to understand. Therefore, this patch moves `WSType` into
`WSScanResult` and share it with `WSRunScanner` with making them friends.
Depends on D68675
Differential Revision: https://phabricator.services.mozilla.com/D68676
--HG--
extra : moz-landing-system : lando
Botond Ballo
Masayuki Nakano
sanketh
Jonathan Kew
Emilio Cobos Álvarez
Cameron McCormack
Makoto Kato
Bogdan Tara
Mike Hommey
Boris Chiou
Mihai Alexandru Michis
Chris Peterson