This patch fixes only the cases if the result of `ComputeIndexOf_Deprecated()`
is used as unsigned integer with implicit or explicit cast.
Differential Revision: https://phabricator.services.mozilla.com/D131336
Offset in a node is declared as "unsigned long" by the standards and we don't
limit node can have less than `INT32_MAX`. So it should return `uint32_t`, but
it also needs to represent the case of "not found". For consistency with some
other APIs like `nsContentUtils::ComparePoints`, using `Maybe` must be a good
style rather than `Result<uint32_t, bool>`.
This patch fixes the callers in assertions for example.
Differential Revision: https://phabricator.services.mozilla.com/D131335
It's hard to fix some callers. Therefore, in this bug, we should fix only
simple cases. Therefore, we should rename existing API first.
Differential Revision: https://phabricator.services.mozilla.com/D131334
They are defined as "unsigned long" by the standards. So we should use
`uint32_t` rather than `int32_t` with the methods. However, layout code
uses `int32_t` a lot for representing the offset. Therefore, this patch
adds `*_FixOffset1` etc for the cases which cannot fix easily in this patch.
Differential Revision: https://phabricator.services.mozilla.com/D131110
It has two methods which take (signed) `long` argument to specify offset in a
DOM node, but it's declared as "unsigned long" in the standards. And now,
they work with rewritten Selection API which take `uint32_t` so that they should
use `unsigned long` for making the handling simpler and safer.
Differential Revision: https://phabricator.services.mozilla.com/D131034
It's an internal API corresponding to `Selection.getRangeAt` DOM API.
I think that it should use `uint32_t` rather than `size_t` because of the
consistency with the DOM API and `Selection::RangeCount()`.
This patch fixes all callers of `GetRangeAt()`, and rewrites it with ranged-
loops unless original ones do not refer `RangeCount()` every time and may run
script in the loop.
Differential Revision: https://phabricator.services.mozilla.com/D128848
Our internal code can touch internal `Selection` object of `<input>` and
`<textarea>`, thus, selection ranges in them may be not in the text node.
Therefore, we should make `TextEditor` instance try to shrink the deleting
ranges into the text node first.
In a followup bug (perhaps, bug 1734846), we should split the deletion code into
`TextEditor` and `HTMLEditor` for making the code simpler and never regressed.
Differential Revision: https://phabricator.services.mozilla.com/D133181
-Wshadow warnings are not enabled globally, so these -Wno-shadow suppressions have no effect. I had intended to enable -Wshadow globally along with these suppressions in some directories (in bug 1272513), but that was blocked by other issues.
There are too many -Wshadow warnings (now over 2000) to realistically fix them all. We should remove all these unnecessary -Wno-shadow flags cluttering many moz.build files.
Differential Revision: https://phabricator.services.mozilla.com/D132289
Similarly, `SplitNodeTransaction::RedoTransaction()` has its own code, but it
does not work well. Let's make it use `HTMLEditor::DoSplitNode()` instead.
This fixes bug 1740656.
Differential Revision: https://phabricator.services.mozilla.com/D132123
`JoinNodeTransaction::UndoTransaction()` has its own splitting code. However,
it has some bugs, it does not handle surrogate pairs correctly and it does not
care selections. `HTMLEditor::DoSplitNode()` is used for splitting a DOM node
from `SplitNodeTransaction::DoTransaction()`. So that we should make
`JoinNodeTransaction::UndoTransaction()` should use `HTMLEditor::DoSplitNode()`
for saving the maintenance cost.
Differential Revision: https://phabricator.services.mozilla.com/D132122
This patch implements `JoinNodesResult` class which callers of
`JoinNodesWithTransaction()` can access removed node, existing (joined) node and
joined point with logical named accessors, and makes
`JoinNodesWithTransaction()` return it.
Then, the callers don't need to change at fixing bug 1735608.
Differential Revision: https://phabricator.services.mozilla.com/D132120
`CreateElementTransaction` is too similar to `InsertNodeTransaction`. Only the
difference is, CreateElementTransaction::DoTransaction() creates an element via
editor instance and marks it dirty before inserting the new element. E.g.,
selection management and whether inserting new node or not at "redo" are same.
So, for the maintenance cost reason, we should get rid of
`CreateElementTransaction`, but for now, we should just make `HTMLEditor` use
`InsertNodeTransaction` instead, with a new pref to make it possible to back it
out the new behavior even after shipping.
Differential Revision: https://phabricator.services.mozilla.com/D132119
This is a different point from `InsertNodeTransaction`. If reference node has
been removed from the DOM tree, it tries to insert the given node to end of
the container instead of returning error. This is an edge case of "redo", and
this is tested by WPT. If we'd need to back it out with Normandy after shipping
this changes, we'd need to uplift WPT expectation change. However, we should
avoid this situation. Therefore, this patch fixes the bug of
`CreateElementTransaction` first.
Differential Revision: https://phabricator.services.mozilla.com/D132118
With this patch, callers of it can handle left and right node logically instead
of assuming that one is split and the other is new one.
Depends on D131748
Differential Revision: https://phabricator.services.mozilla.com/D131749
First, left/right node accessors are not used so that we can get rid of them.
However, we should have similar methods which can retrieve original node and
new node. Therefore, this adds `GetNewContent()` and `GetOriginalContent()`.
Next, `SplitPoint()` should return both `EditorDOMPoint` and `EditorRawDOMPoint`
for avoiding unnecessary conversion from `EditorRawDOMPoint` to
`EditorDOMPoint`. Therefore, this patch makes it a template method too.
Finally, this patch adds helper methods to get point of each related content.
E.g., `AtNewContent()` corresponding to `GetNewContent()`.
Differential Revision: https://phabricator.services.mozilla.com/D131748
With this patch, we stop registering `Alt+A` as a shortcut key for "Select All" on Linux and register `Ctrl+A` instead, which is the default shortcut for the command on other GTK applications, Chromium, and Windows. `Alt+A` was also causing a bug for menubar navigation because it hijacks `Alt+A` access key. This patch does not stop Linux environments from registering `Alt+A` as a shortcut key for "Select All"; it just drops Gecko's additional, non-standard shortcut key definition, and defaults to `Ctrl+A` instead.
Differential Revision: https://phabricator.services.mozilla.com/D131062
There are a lot of places which calls
`HTMLEditor::MaybeSplitAncestorsForInsertWithTransaction()` and then calls
`HTMLEditor::CreateAndInsertElementWithTransaction()` with split point of the
preceding result. It's better to wrap them in a method for reducing the
maintenance cost.
`HTMLEditor::CreateOrChangeBlockContainerElement()` handles to calling them
safer than the others so that let's use this code for the new method. The
additional change won't change the most web apps which don't touch the DOM
tree from legacy mutation event listeners.
Depends on D131198
Differential Revision: https://phabricator.services.mozilla.com/D131199
I realized that it's now used only by `HTMLEditor` so that we can move it
into `HTMLEditor` and we can make `CreateElementTransaction` treat `HTMLEditor`
directly rather than via `EditorBase`.
Differential Revision: https://phabricator.services.mozilla.com/D131198
It touches the DOM tree only with `SplitNodeTransaction()` and it now returns
`NS_ERROR_EDITOR_DESTROYED` so that the callers don't need to check whether
the editor is destroyed or alive by themselves.
Depends on D131043
Differential Revision: https://phabricator.services.mozilla.com/D131044
For making the code simpler, this patch makes it returns
`Result<nsCOMPtr<nsIContent>, nsresult>` instead of using `ErrorResult`.
This must make the construction faster and callers simpler.
Depends on D130950
Differential Revision: https://phabricator.services.mozilla.com/D131043
It does the following things when caret is collapsed in a text node in a `<p>`
or `<div>` element.
1. Split the text node containing caret to insert `<br>` element
2. Insert `<br>` element after it
3. Split ancestor elements which inclusive descendants of the `<p>` or `<div>`
4. Delete the `<br>` element if unnecessary from the left paragraph
#3 and #4 are performed by `HTMLEditor::SplitParagraph()` and it calls
`WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement()` correctly before
splitting the block. However, in the case (caret is at middle of a text node),
the text has already been split to 2 nodes because of #1. Therefore, it fails
to handle to keep the white-space visibility.
So that I believe that the root cause of this bug is, the method does much
complicated things which are required, and doing the redundant things will
eat memory space due to undo transactions. However, for now, I'd like to fix
this with a simple patch which just call the preparation method before splitting
the text node because I'd like to uplift this if it'd be approved (Note that
this is not a recent regression, the root cause was created by bug 92686 which
was fixed in 17 years ago:
<https://searchfox.org/mozilla-central/commit/2e66280faef73e9be218e00758d4eb738395ac83>,
but must be annoying bug for users who see this frequently).
The new WPTs are pass in Chrome.
Differential Revision: https://phabricator.services.mozilla.com/D130950
Similar to the previous patch, this patch also make it track invisible
white-space ranges and clear outdated things.
And this makes it cache some information instead of tracking some changes
because of performance reason.
Depends on D131037
Differential Revision: https://phabricator.services.mozilla.com/D131038
It `pointToSplit` should be tracked at replacing text, but I have no idea how
to test this because it replaces the text after the split point.
Differential Revision: https://phabricator.services.mozilla.com/D131036
Currently, it assumes that new node is the left node and split node is the
right node. However, we need to make it possible to handle the case that
right node is new one for fixing bug 1735608.
Depends on D130458
Differential Revision: https://phabricator.services.mozilla.com/D130624
With the rewrite, we reduce the dependency of lwbrk LineBreaker::Next(),
WordBreaker::Next(), and their `int32_t` sentinel values
NS_LINEBREAKER_NEED_MORE_TEXT and NS_WORDBREAKER_NEED_MORE_TEXT.
Differential Revision: https://phabricator.services.mozilla.com/D130392
There is 2 overloads. One is `TextServicesDocument`'s method, and the latter is
for `nsIEditActionListener`. The latter could occur if `TextServicesDocument`
is added to the editor when there is no inline spellchecker, `EditorSpellCheck`
or `mozSpellChecker`.
https://searchfox.org/mozilla-central/rev/d2f8488b6a704443a5c5bfc6d2878171b5f0d393/editor/libeditor/EditorBase.cpp#2388,2391,2393,2396,2398
I don't know whether this is possible case, but unfortunately,
`nsIEditActionListener::DidJoinNodes()` is not implemented by JS and implemented
only by `TextServicesDocument`. Therefore, we can make it "noscript" and use
non-scriptable classes as arguments.
This patch makes them get jointed point and removed content node and the joining
direction for fixing bug 1735608. Unfortunately, there is no test framework
to check the result in `TextServicesDocument` nor the new path. The latter
should be tested when we fix bug 1735608 later.
Differential Revision: https://phabricator.services.mozilla.com/D130458
With the rewrite, we reduce the dependency of lwbrk LineBreaker::Next(),
WordBreaker::Next(), and their `int32_t` sentinel values
NS_LINEBREAKER_NEED_MORE_TEXT and NS_WORDBREAKER_NEED_MORE_TEXT.
Differential Revision: https://phabricator.services.mozilla.com/D130392
Currently, it assumes that `aLeftNode` is new node and the other is the original
one. However, we need the opposite case when we fix bug 1735608.
Unfortunately, we cannot test the new path until we fix bug 1735608. Therefore,
the new path may contain some bugs.
Differential Revision: https://phabricator.services.mozilla.com/D130427
Currently, it assumes that all children or text data of `aRightContent` is
moved into `aLeftContent`. However, we need the opposite case when we fix
bug 1735608.
Unfortunately, we cannot test the new path until we fix bug 1735608. Therefore,
the new path may contain some bugs.
Differential Revision: https://phabricator.services.mozilla.com/D130426
The telemetry result is written in bug 1706266 comment 1:
https://bugzilla.mozilla.org/show_bug.cgi?id=1706266#c1
`increasefontsize`, `decreasefontsize`, `gethtml`, `heading` and `readonly` are
obviously not used by web apps in the wild. Therefore, they can be disabled
in all channels.
`contentReadOnly` and `insertBrOnReturn` are odd. The usage is really low (less
than 1% of beta users). However, the number of documents which used the command
is about 1k samples. The result of the commands are not tiny (making the editor
not editable or changing the behavior at typing `Enter` key in `<div>`, `<p>`,
etc). Therefore, it's hard to use them in web apps which supports not only
Gecko. So I guess that they are collected the number of used by automated
tests of somebody because of the constant number in other beta versions.
Perhaps, we should disable it only in Nightly channel for now, and after a
couple of releases, we should try to disable those commands too later.
Depends on D130328
Differential Revision: https://phabricator.services.mozilla.com/D130329
I'd like to split utilities for all editors and only for `HTMLEditor`.
This moves some trivial classes into new `HTMLEditHelpers.h`. Perhaps, it's
better if we can move them into `HTMLEditUtils.h`, but it has too many inline
method definitions so that adding new classes into it makes it much bigger.
Depends on D130349
Differential Revision: https://phabricator.services.mozilla.com/D130425
Now, it does not notify nobody before joining the nodes. Therefore, it can
return error immediately if it fails to create a transaction to join the given
nodes.
Additionally, this patch makes it return `NS_ERROR_EDITOR_DESTROYED` if the
editor is destroyed while it handles to join nodes. Therefore, we can get rid
of the check by the callers.
Differential Revision: https://phabricator.services.mozilla.com/D130349
Masayuki Nakano
Chris Peterson
Richard Marti
Dan Minor
Sandor Molnar
Greg Tatum
Ting-Yu Lin
Iulian Moraru
ssummar