Even though the method returns only in specific cases, but the result affects
only one caller, `HTMLEditor::GetNextSelectedTableCellElement()`. Therefore,
we can create new generic utility method,
`HTMLEditUtils::GetTableCellElementIfOnlyOneSelected()` and get rid of
`HTMLEditor::GetCellFromRange()`.
Note that the warnings in `HTMLEditor::GetCellFromRange()` is just noise for
any callers. So, this gets rid of the useless spam warnings.
Differential Revision: https://phabricator.services.mozilla.com/D72586
As an alternative of `HTMLEditor::GetCellFromRange()`, this patch creates more
generic utility method.
Depends on D72296
Differential Revision: https://phabricator.services.mozilla.com/D72585
Currently, `EditorBase::ExtendSelectionForDelete()` depends on some
`nsISelectionController` methods to compute extended range for deletion
from collapsed selection. They are implemented by
`nsFrameSelection::MoveCaret()` and `nsFrameSelection::TakeFocus()`.
Ideally, we should split these methods for computation part and performing
part. However, they change selection with updating other selection state,
for example, table selection state and bidi information. Therefore, it's
impossible to split them with simple code. However, I need to change
`EditorBase::ExtendSelectionForDelete()` just return extended range.
Therefore, this patch creates `nsFrameSelection::PeekOffsetForCaretMove()`
which has the main path in `MoveCaret()` for the `EditorBase` method.
Then, `MoveCaret()` and new `nsFrameSelection::CreateRangeExtendedToSomewhere()`
share the computation code of expanding normal selection.
Finally, this patch wraps `nsFrameSelection::CreateRangeExtendedToSomewhere()`
with new public inline methods for `EditorBase`.
The following patch will remove no-user methods of `nsISelectionController`.
Differential Revision: https://phabricator.services.mozilla.com/D72295
They are used only by `HTMLEditor` so that we should hide them from
`TextEditor` for making it clearer that they are not used by `TextEditor`.
Note that there are 2 `DeleteNodeWithTransaction()` in `HTMLEditor` class.
One is `EditorBase`'s method and the other is `HTMLEditor`'s method.
`HTMLEditor`'s one is check whether the removing node is editable or not,
but in some cases, we need to move non-editable nodes. Therefore, this
patch makes `ReplaceContainerWithTransaction()` call `EditorBase`'s one
for keeping current behavior.
Differential Revision: https://phabricator.services.mozilla.com/D72822
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
