It should be treated as `uint32_t` since DOM API does so. However, there are
some exceptions:
* Result of `nsINode::ComputeIndexOf()`
* Result of `nsAString` methods
They return `-1` as not found, and anyway, they cannot treat large integer
than `INT32_MAX`. Therefore, this patch does not touch around them.
Differential Revision: https://phabricator.services.mozilla.com/D118933
It's a static method and called only with `EditorDOMPointBase` instances.
So, we can add new API to `EditorDOMPointBase` instead.
Note that it's changed to return container when the container is a data node
rather than a text node. This should be better because no data node can
have children.
Depends on D117382
Differential Revision: https://phabricator.services.mozilla.com/D117490
This is causes taking back bugs in
`WSRunScanner::GetNewInvisibleLeadingWhiteSpaceRangeIfSplittingAt()` and
`WSRunScanner::GetNewInvisibleTrailingWhiteSpaceRangeIfSplittingAt()` (I.e.,
they were fixed by bug 1647556). Therefore, this removes the odd `if` blocks
which are pointed with `XXX` comment for avoiding "regressions" (without them,
some WPTs for `InputEvents.getTargetRanges()` become failure).
Depends on D89582
Differential Revision: https://phabricator.services.mozilla.com/D89869
The editor modules does QI too many times when it sets or removes some style
with `execCommand` or XPCOM API. Therefore, there should be an API to
retrieve `nsStyledElement` pointer from `nsINode*`.
Differential Revision: https://phabricator.services.mozilla.com/D87990
It may be faster to use `AutoTrackDOMRange` directly. Therefore, current
`WhiteSpaceVisibilityKeeper::PrepareToDeleteRange()` should be renamed to
`WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints()` and
we should make `HTMLEditor::HandleDeleteNonCollapsedSelection()` track
the range by itself.
Depends on D85846
Differential Revision: https://phabricator.services.mozilla.com/D85847
The difference between `HTMLEditor::HandleDeleteCollapsedSelectionAtTextNode()`
and `HTMLEditor::HandleDeleteCollapsedSelectionAtWhiteSpace()` is whether
it treats surrogate pair or not. And it's easy to handle it in `WSRunScanner`.
Therefore, they can handle all deletion in a text node.
Some new failures are caused by that we normalize preceding white-spaces of
deletion range but Blink does not to it. These failures will be fixed by
part 5.
Depends on D84946
Differential Revision: https://phabricator.services.mozilla.com/D84947
`InputEvent.getTargetRange()` should have actual delete range. It means that
the range should contain the invisible leading/trailing white-spaces which
need to be removed for keep them invisible, but should not contain the range
of normalizing white-space sequence because they are not target of edit action
indicated by `InputEvent.inputType`.
So, we can use new path which uses the new white-space normalizer for
computing the value of `InputEvent.getTargetRanges()` because difference of
white-space normalizer shouldn't affect the deleting ranges (although, some
existing path calls `DeleteNodeIfInvisibleAndEditableTextNode()` later so that
the new method, `ComputeRangeInTextNodesContainingInvisibleWhiteSpaces()`, does
not exactly same thing, but the result shouldn't become different in usual
cases). This new path can test with some WPTs under `editing/other`.
This patch creates new backspace/delete key handler when caret is at next
to a white-space as `HTMLEditor::HandleDeleteTextAroundCollapsedSelection()`
and creates helper methods of `WSRunScanner` to treat invisible leading and
trailing white-spaces.
Note that new failures are caused by the difference whether adjacent white-space
sequence at deletion is normalized or not in edge cases. They will be fixed
by the part.5.
Depends on D84943
Differential Revision: https://phabricator.services.mozilla.com/D84944
With this patch, the creation cost of `TextFragmentData` is minimized, but
`MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()` can work
with the latest DOM tree information at touching start of the deleting range
even after touching end of the range.
Depends on D83228
Differential Revision: https://phabricator.services.mozilla.com/D83229
For representing delete or replace data which have the range and replace string,
this patch creates `ReplaceRangeDataBase` class which is stack-only, has
`EditorDOMRange` or `EditorDOMRangeInTexts` as the range and has replace string
which can be empty string.
Then, `MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()` should
modify the DOM tree by itself. With the following patch, the behavior is
corrected for multiple DOM tree changes.
Depends on D83224
Differential Revision: https://phabricator.services.mozilla.com/D83226
It removes some invisible leading and/or trailing white-spaces when it inserts
`<br>` element into the invisible white-space sequence. It currently checks
whether the insertion point is in invisible leading and trailing white-spaces
or not with `FindNearestFragment()`, but we can do same thing with
comparing the insertion point with the result of
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()` and
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()`. However, current
implementation does not make sense because:
- It checks trailing white-spaces with `!IsEndOfHardLine()` and
`IsStartOfHardLine()`, but this means that it does ignores invisible
white-spaces which are the only content in a line.
- It checks leading white-spaces with `!IsStartOfHardLine()` and
`IsEndOfHardLine()`, so, this also ignores invisible white-spaces which
are the only content in a line.
- The important thing of the logic is prevent that invisible leading and
trailing white-spaces become visible with new `<br>` element, but this
is done only for trailing white-spaces.
Differential Revision: https://phabricator.services.mozilla.com/D82283
One of the `WSFragment`users' purpose is, they remove invisible white-spaces
when there are. So, `TextFragmentData` should have API to retrieve the
ranges and use them for initializing `WSFragment` which represents leading
or trailing white-spaces. For making this patch smaller as far as possible,
these APIs implements only the case when there is no NBSP.
For result of the new API, this creates a template class, `EditorDOMRangeBase`,
whose boundary type is `EditorDOMPointBase`. Its methods are named from
`nsRange`'s same methods.
Differential Revision: https://phabricator.services.mozilla.com/D82272
This patch tries to implement Blink-compat white-space normalizer for
`HTMLEditor`.
It's difficult to list up our traditional white-space normalization rules
because `WSRunObject` touches white space sequence only when there is not
acceptable case, e.g., an ASCII white-spaces will be adjacent to another
one, and replaces only unacceptable white-space only. Therefore, whether
white-space sequence may start with either an ASCII white-space or an NBSP.
On the other hand, Blink and WebKit makes white-space sequence always
starts with an NBSP or an ASCII white-space (unfortunately, they behave
differently!). So, for web-compat, we should simulate Blink's behavior
because either behavior is reasonable but Blink have more market share.
This patch simply adds new white-space normalization path for the new one,
and it's switchable with a pref, and still disabled by default.
The other reason why we should do this is, our traditional white-space
normalizer touches the DOM a lot of times per edit action, and the timing
is both before and after touches the DOM tree. Therefore, it's difficult
to compute actual deleting range for `InputEvent.getTargetRanges()` and
touching a lot of times causes running mutation event listeners a lot and
creates a lot of transaction class instances. So, new one have a lot of
merits:
1. Improve web-compat
2. Improve the peformance
3. Improve the security
4. Improve the footprint (but this is now worse then traditional one)
5. Simplify the implementation
The new normalizer is mostly implemented with only 3 `HTMLEditor` methods.
One is `HTMLEditor::DeleteTextAndNormalizeSurroundingWhiteSpaces()`. This is
semi-public method for the edit action handlers. This takes a range with
2 `EditorDOMPoinInText` to delete the range simply. This also replaces
surrounding white-space sequence if necessary. For inserting text case,
this method also handles only white-space normalization when it's called
with collapsed range, i.e., same `EditorDOMPointInText`. This tries to use
`RepaceTextWithTransaction()` as far as possible to reduce creation cost of
transaction classes and the footprint.
Another one is `HTMLEditor::ExtendRangeToDeleteWithNormalizingWhiteSpaces()`.
This tries to extend the given range to normalize surrounding white-spaces.
This is currently not optimized for footprint because this may include
white-spaces which do not need to be replaced. This optimization should be
done before shipping, but for now, enabling `InputEvent.getTargetRanges()` in
Nightly channel is more important. So that it should be done in a follow-up
bug.
The other is `HTMLEditor::GenerateWhitepaceSequence()`. This creates
normalized white-space sequence with surrounding character information.
For keeping this method simple as far as possible, we shouldn't optimize
the range of generation even in follow-ups.
Finally, the white-space sequence is not tested in mochitests, so that we
can enable this new normalizer when we run mochitests under
`editor/libeditor/tests`. However, WPT has some tests. We should keep
them running with current normalizer for checking regression. Instead,
we should enable the pref only for the new WPT added by the previous patch.
Depends on D78655
Differential Revision: https://phabricator.services.mozilla.com/D78656
`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
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
With adding new type, `EditorDOMPointInText` whose container is
`RefPtr<dom::Text>`, we can replace `WSRunScanner::WSPoint` and make
`WSRunScanner` and `WSRunObject` can use its various API. Then, this
patch adds a lot of `NS_ASSERTION`s which can detect existing bugs.
Note that it may look like redundant that `EditorDOMPointInText::IsChar*()`
requires `EditorDOMPointInText::IsEndOfContainer()` check before that.
However, this makes what the callers check clearer.
Differential Revision: https://phabricator.services.mozilla.com/D64336
--HG--
extra : moz-landing-system : lando
Some methods of `EditorDOMPointBase` assumes the container node type is
`nsINode`. However, it's not good for reuse. Therefore, this patch makes
most methods of them be template methods.
Differential Revision: https://phabricator.services.mozilla.com/D64331
--HG--
extra : moz-landing-system : lando
They are really messy because they take a lot of out parameters, and these
out parameter meaning is really unclear. Therefore, they should return
a stack only class instance which explain the meaning with getter methods.
And also it should store the result node as `nsIContent`.
And also this patch adds static methods for them for their users which don't
need `WSRunScanner` instance.
Differential Revision: https://phabricator.services.mozilla.com/D63613
--HG--
extra : moz-landing-system : lando
Additionally, `WSRunObject::ScrabBlockBoundary()` and
`WSRunObject::PreparetToJoinBlocks()` are used only the it. Therefore, we
can clean them up.
Differential Revision: https://phabricator.services.mozilla.com/D44201
--HG--
extra : moz-landing-system : lando
For avoiding confusion between API of `nsRange` and `StaticRange`, I'd like to
rename `nsRange::CreateRange()` to `nsRange::Create()` because
`StaticRange::CreateStaticRange()` is too long name and
`StaticRange::CreateRange()` sounds odd. This patch renames it and changes
related methods to template methods to avoid runtime cost of temporary
`RawRangeBoundary` instance creation.
Differential Revision: https://phabricator.services.mozilla.com/D35141
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::ApplyBlockStyle()` stores `curBlock` and `newBlock` during its
loop to keep handling from deeper child to ancestor, and may do two things for
a `curNode`. If `curBlock` and/or `newBlock` is moved from expected container
when it sets one of or both of them, this patch check whether mutation event
listeners change the DOM tree. Additionally, this patch also checks whether
`curNode' is moved by mutation event listener at first step of two jobs for it.
Differential Revision: https://phabricator.services.mozilla.com/D32689
--HG--
extra : moz-landing-system : lando
This is a simple mistake of `MOZ_ASSERTION()` in it. When `mParent` is a node
which can have children, `mChild` should be non-`nullptr` or `mOffset` should
/ be set to the end of `mParent`. But when `mParent` is not a container, any
`mOffset` value should be allowed.
Differential Revision: https://phabricator.services.mozilla.com/D30014
--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
HTMLEditor::InsertElementAtSelection() doesn't check if selection is in
native anonymous subtree. Therefore, it could insert element into <input>
element etc.
This patch adds new API to EditorDOMPointBase to compute ancestor point which
is not in native-anonymous subtree and make
HTMLEditor::GetBetterInsertionPointFor() use it to not return point in any
native-anonymous subtrees.
This method is not a virtual call, and also looks nicer.
This patch was mostly generated by a Python script, but I manually
cleaned up the code in a few places where statements didn't need to be
split across multiple lines any more.
MozReview-Commit-ID: 8JExxqSRc59
--HG--
extra : rebase_source : df6330a89e8d65dfe7a6fda0c8cb9f9732302efc
A lot of methods take |const EditorRawDOMPoint&| as their argument. However,
some of them are called with EditorDOMPoint::AsRaw(). This is not good for
performance because:
1. Needs to create temporary instance of EditorRawDOMPoint.
2. EditorRawDOMPoint::AsRaw() may be used by simple mistake.
3. Such methods may call EditorRawDOMPoint::Offset(), however, it's not copied
to the original EditorDOMPoint instance. So, callers may need to compute
offset again.
So, such methods should be changed to template methods if they are not virtual
method and AsRaw() should be gotten rid of for prevent it looking not expensive.
MozReview-Commit-ID: DAqqW4a2EMS
--HG--
extra : rebase_source : 120b920477fe6e7231271411a00994325acdb842
Editor(Raw)DOMPoint::IsEndOfContainer() checks mIsChildInitialized before
referring mChild. However, it may be true even if mParent is not a container
node like a text node. Therefore, if mParent is a text node and
mIsChildInitialized is true, it always returns true (i.e., even if mOffset
isn't same as length of mParent).
This patch makes it check mParent->IsContainerNode() before checking
mIsChildInitialized because after checking mIsChildInitialized, it validates
the relation of the members. So, this keeps the validation simple.
MozReview-Commit-ID: K2XrAZoNv2I
--HG--
extra : rebase_source : cba8190f199ab2e9d99547948ab50f0cc8c8ff88
EditorBase::JoinNodeDeep() returns a DOM point which was start of right node.
Currently, this is set before actually joining the nodes. Therefore, we see
warnings since right node which becomes joined node may have less children
than left node.
This patch also makes the NS_WARNING_ASSERTION to NS_ASSERTION since no tests
hit this. So, we can use it to detect regressions.
MozReview-Commit-ID: 3RmRP588AkF
--HG--
extra : rebase_source : 2cab458ffac566c81eb8d3f47e755ffacb1553d2
This patch only fixes warning, not changing actual behavior of editor.
HTMLEditRules::ReturnInParagraph() splits paragraph *around* given point.
Therefore, from point of view of caller, offset of setting point may be
invalid after HTMLEditRules::ReturnInParagraph() handled the edit action.
In this case, invalidating stored child of the point may cause warning
since offset may be larger than length of its container.
So, if HTMLEditRules::ReturnInParagraph() handles the edit action,
the caller, HTMLEditRules::WillInsertBreak(), should cancel invalidating
the stored child for avoiding unnecessary warning.
MozReview-Commit-ID: DKJlr0Awwlo
--HG--
extra : rebase_source : 8fbefff3b46e55daf8db6342f00de904315d7fcc
EditorDOMPointBase::RewindOffset() treats it as error when offset is
same as length of container. However, this is wrong because offset can
be same as length of its container since it means after the last child
or last character. So, if an instance points end of a container node,
Rewind() always fails unexpectedly.
MozReview-Commit-ID: A9bvsxETDoo
--HG--
extra : rebase_source : f05994f2924adaa29711da3916f431688eaa79b7
When Enter key is pressed at start or end of <a href="foo"> element, we
shouldn't split it (in other words, we shouldn't create empty <a href="foo">
element in new paragraph) because users must not want to keep editing *same*
link in new paragraph in most cases.
This patch adjusts HTMLEditRules::ReturnInParagraph() selection start point
locally when it gets selection. If caret is at start of an <a href="foo">
element, moves caret to before the element. If caret is at end of an
<a href="foo"> element, moves to after the element.
MozReview-Commit-ID: 3L3eDzc6Dk
--HG--
extra : rebase_source : dfb22486d2709e84ef7ec1eee3a9bfdef11cbe99
Different from RangeBoundaryBase, EditorDOMPointBase can store only child node.
I.e., mOffset may be invalid until its Offset() is called. So,
GetChildAtOffset() isn't good name. It should be just GetChild().
Similarly, GetNextSiblingOfChildAtOffset() and
GetPreviousSiblingOfChildAtOffset() should be renamed to GetNextSiblingOfChild()
and GetPreviousSiblingOfChild().
MozReview-Commit-ID: WpkPmDwkrL
--HG--
extra : rebase_source : 2be1fcbee129f4c96e9b166be5a924845340708f
EditorDOMPointBase::Container() may return nullptr. So, it should be renamed
to GetContainer().
Then, the method name becomes longer and a lot of callers access the result
directly. So, there should be following methods to make the callers shorter:
- GetContainerAsContent() to return the container as nsIContent*.
- GetContainerAsElement() to return the container as dom::Element*.
- GetContainerAsText() to return the container as dom::Text.
- GetContainerAsDOMNode() to return the container as nsIDOMNode*.
- CanContainerHaveChildren() to check if the container can have child nodes.
- IsInDataNode() to check if the point is in a data node including text node.
- IsInTextNode() to check if the point is in a text node.
- IsContainerHTMLElement() to check if the container is specific HTML element.
- IsContainerAnyOfHTMLElements() to check if the container is one of the
specified HTML elements.
MozReview-Commit-ID: LkN2WBbCPj0
--HG--
extra : rebase_source : 9da1e70d4c74f7ad573e244cf6c7b21a0c7b4410
Editor code sometimes sets a DOM point to end of a node. In this case, we
need to write |Set(node, node->Length())|. So, it should have
|void SetToEndOf(const nsINode* aContainer)| for making meaning of the code
clearer.
MozReview-Commit-ID: 91shMCD2d84
--HG--
extra : rebase_source : 3401af50aabc1b677deeae664e8c56506182d28c
HTMLEditor::NormalizeEOLInsertPosition() takes a set of container node and
offset in it for specifying insertion point. So, this should be replaced
with |const EditorRawDOMPoint&| and it should return |EditorDOMPoint| rather
than modifying the argument.
Additionally, perhaps, GetBetterInsertionPointFor() is better name for it.
MozReview-Commit-ID: IB1FhrkzK2G
--HG--
extra : rebase_source : 55f8c1d9fa1f149e81114c929f6e6232aba03905
HTMLEditor::InsertNodeAtPoint() should take |const EditorRawDOMPoint&| as
an argument which specifies point to insert. Additionally, it should take
|EditorDOMPoint*| to return the next point of actual insertion point.
Additionally, this patch renames it to InsertNodeAtProperAncestor() for
explaining what it will do.
MozReview-Commit-ID: HYUzSlyPxAd
--HG--
extra : rebase_source : 57033cdc4458b45a5fe9d6aa642c185133a79304
Masayuki Nakano
Mirko Brodesser
Makoto Kato
Sylvestre Ledru
Emilio Cobos Álvarez
Boris Zbarsky
Andrew McCreight
Andrea Marchesini