While moving children of a container to another container, mutation observer
may move children before moving and/or move node immediately after the
insertion point. Therefore, EditorBase should store all children which
should be moved with a local variable. Then, move one by one carefully.
E.g., if a child before being moved is moved to different container, it
shouldn't be moved because JS already handles it as expected for the web
app. If next sibling of the insertion point has been moved, EditorBase
should stop moving the remaining children because it may move children to
odd position and it may cause dataloss.
This patch creates EditorBase::MoveChildren() and it moves children carefully
with above checks. Additionally, making its callers simpler, this patch also
creates EditorBase::MovePreviousSiblings() and MoveAllChildren().
MozReview-Commit-ID: FJDdSqXIlqD
--HG--
extra : rebase_source : 54bded29dbbd9547339a2c6e1a1264e22fbdc740
CreateElementTransaction and SplitNodeTransaction store DOM point with
RangeBoundary now. However, it refers next child of referring point.
Therefore, if it's moved to different container or removed from the DOM
tree, they cannot do undo/redo. We can say for the child node, however,
it doesn't make sense depending on a node which is not directly referring.
MozReview-Commit-ID: Baohbub3RNZ
--HG--
extra : rebase_source : 6776420271cdaf91aae6213fb6e1bcf1841112b1
EditorDOMPointBase should store child node at offset directly rather than
previous sibling of child node at offset because if referring child node or
its previous sibling is moved to different DOM point,
EditorDOMPointBase::GetChildAtOffset() may return different node. However,
users of this class assumes that after it stores child node at offset, it'll
return same node.
So, EditorDOMPointBase should store child node instead of "ref". Additionally,
when it stores nullptr as the child, it should set mOffset to length of the
mContainer because it means it refers after the last child.
MozReview-Commit-ID: 7ZdkwKLJEjo
--HG--
extra : rebase_source : c2a0a1be1285466e5fa96162779e86c37ae78655
EditorDOMPointBase should store child at offset for editor uses. E.g., when
editor wants to refer a node as child node of an EditorDOMPoint instance,
even if the node is unexpectedly moved to different container, editor wants
to keep referring the child node rather than its previous sibling.
Therefore this patch makes EditorDOMPointBase not a sub class of
RangeBoundaryBase but copying all methods and members of RangeBoundaryBase
into EditorDOMPointBase for keeping current behavior completely.
MozReview-Commit-ID: LIyPFkCfsZ9
--HG--
extra : rebase_source : fd76c4808625f8f8a86f7b4e4c1ac22fbdc11dd5
There are 3 spam warning assertion in EditorDOMPointBase.
One is in constructor. Which warns if IsSet() returns false, but it checks
before finishing initializing.
The others are in IsStartOfContainer() and IsEndOfContainer(). They try to
check if mOffset is valid value with current DOM tree if it's initialized
when it's at start or end of the container. However, they do it even when
it's not start nor end of the container.
This patch fixes those spammers.
MozReview-Commit-ID: DvFa501m9V0
--HG--
extra : rebase_source : c97ac371054a54eabc07b3ec726b924d6be61e25
No one uses nsIInlineSpellChecker.spellCheckAfterEditorChange from script.
So I think we can mark this interface as noscript.
Since this method is scriptable, we need QI to get nsIDOMNode. If we can
change to noscript, it can reduce QI to get nsIDOMNode.
MozReview-Commit-ID: GC0WuFyTlaZ
--HG--
extra : rebase_source : 16ca9fc548e86747ac17407be48295c709174fb5
Part 1. moves native dom method of CreateBR, so we should use it for
CreateMozBR, and use native dom as parameter.
MozReview-Commit-ID: DHUB88HfowQ
--HG--
extra : rebase_source : 00fefd12b50bef0f1a9bd69e1b7992a19999d42f
One of CreateBR is still virtual method, but it is unnecessary to use it.
So we should remove virtual keyword and remove override method.
Also, we should move native dom version of CreateBR to TextEditor to use it
on TextEdtitor.
MozReview-Commit-ID: GCazJtY4urV
--HG--
extra : rebase_source : 8e0d71631f0070a928bc0f4817dd6efe7c833f1b
When TextEditor::CreateBRImpl() splits a text node before inserting new <br>
element, it initializes pointToInsertBrNode only with the right text node.
Then, it refers its Offset() after inserting new <br> node before the point.
Therefore, the offset is computed with the new DOM tree. So, adding 1 to
the offset is redundant only in this case.
So, before calling CreateNode(), it needs to make pointToInsertBrNode store
offset with calling its Offset().
Note that this ugly code will be replaced with patches for bug 1408227.
Additionally, this doesn't use AutoEditorDOMPointChildInvalidator because
it's not available in 58 but we need to uplift this patch.
Finally, I'm not sure how to check this in automated tests. Therefore, this
patch doesn't include automated tests.
MozReview-Commit-ID: IaQBonoGawR
--HG--
extra : rebase_source : a89559932f27d98a02baf3e207c6be3c2a545aad
HTMLEditRules::MaybeSplitAncestorsForInsert() may be called with a point in a
text and it needs to return given split point as is. Additionally, the given
point may be in a text node. So, it may not be represented with an
nsCOMPtr<nsIContent>.
Therefore, we need to add new member, EditorDOMPoint, to SplitNodeResult and
when MaybeSplitAncestorsForInsert() needs to return the given point as is,
it should use it.
Note that if the methods which return SplitNodeResult split some nodes actually,
the left node and/or the right node may be removed from the DOM tree. In this
case, EditorDOMPoint cannot store such orphan node. Therefore, we cannot
make mNextNode nor mPreviousNode EditorDOMPoint.
MozReview-Commit-ID: LwH8RZzkrmT
--HG--
extra : rebase_source : a5ae2328bef3d887c0bf4e1b9c4a4247b93a4ac0
Now, we can make HTMLEditRules::SplitAsNeeded() use |SplitNodeResult| as its
result and |const EditorRawDOMPoint&| as specifying start of the deepest right
node. Then, the implementation becomes simpler.
And I think that we should rename it to MaybeSplitAncestorsForInsert().
Additionally, this patch makes it stop calling
EditorBase::IsDescendantOfEditorRoot() and HTMLEditor::GetActiveEditingHost()
because they are really expensive. Instead, it should check if the given start
point of the deepest right node is in active editing host before the loop and
if the loop reaches editing host.
MozReview-Commit-ID: KKpj5uyT2J
--HG--
extra : rebase_source : 0c9e9e9e28505b0fb5752e1cd4d42f64d22af3e7
In the following patch, we need to invalidate offset a lot of places after
splitting nodes. Therefore, there should be a helper stack class before that.
MozReview-Commit-ID: BgijAU7OizU
--HG--
extra : rebase_source : 520f29dacdffe5f7137ba7f11b289241b5fbface
First of all, this patches fixes a bug of EditorBase::CreateNode(). It takes
|EditorRawDOMPoint&| but it should be |const EditorRawDOMPoint&| for making
callers can specify temporary instances.
Next, this patch creates |SplitNodeResult| stack class for result of
EditorBase::SplitNodeDeep(). SplitNodeDeep() needs to return previous node
and next node of split point. They are called as left node and right node,
but these calls are really different term usage. Therefore, this patch names:
aOutLeftNode -> SplitNodeResult::GetPreviousNode()
aOutRightNode -> SplitNodeResult::GetNextNode()
and also declares SplitNodeResult::GetLeftNode() and
SplitNodeResult::GetRightNode() which are same meaning as left/right node of
other splitting methods.
Additionally, of course, this patch makes SplitNodeDeep() use
|const EditorRawDOMPoint&| to receive the start point of right node.
MozReview-Commit-ID: FnJpeKgtzm4
--HG--
extra : rebase_source : 3829e5528ef837b13fed305e0df1dbbf00e02a07
Make the loop in EditorBase::SplitNodeDeep() use EditorDOMPoint for making the
code simpler.
MozReview-Commit-ID: 3do3rWV4eIh
--HG--
extra : rebase_source : 480a5b5b133d8a735bda6ddec07e4edf9ef34035
Now, we can merge two if blocks in the loop of EditorBase::SplitNodeDeep() and
get rid of |didSplit| variable.
MozReview-Commit-ID: LJZHF6x2GLR
--HG--
extra : rebase_source : 5f58508f6fc0928236252670c85383a13200fc2c
This doesn't change any meaning of the loop.
It is a bug if the loop meets orphan node before meeting the most ancestor
node to be split which is given as aNode. So, we can check it before trying
to split it.
MozReview-Commit-ID: 1TD7WHCoZh1
--HG--
extra : rebase_source : 17b8d7b3db458e29fb52be5cedb900560e1b70a4
EmptyContainers::yes and EmptyContainers::no are not so clear name what they
mean.
They means whether NodeSplitDeep() creates or won't create empty nodes when
split point is at start edge or end edge of an element.
This patch renames them to SplitAtEdges::eDoNotCreateEmptyContainer and
SplitAtEdges::eAllowToCreateEmptyContainer and make
HTMLEditor::InsertNodeAtPoint() use it instead of an bool argument.
Additionally, the argument of NodeSplitDeep() is now not optional because
this difference is really important when you read around the callers.
MozReview-Commit-ID: 9hpMRkVDvCg
--HG--
extra : rebase_source : ee892361d66c9c9c5ed45ee9d3321474257ac417
Although, we need to make WSRunObject::PrepareToSplitAcrossBlocks() in
another bug, we should do it now for making HTMLEditRules::ReturnInParagraph()
implementation simpler.
MozReview-Commit-ID: AoMYqAEgCaV
--HG--
extra : rebase_source : cdfb16379f4ecab2a2694aeb283edd111fc81e95
nsIEditActionListner::DidSplitNode() takes 4 arguments, the right node,
old offset in the old right node before splitting, the new left node and
nsresult.
Computing offset for this doesn't make sense because it's always same as
the length of the left node. Additionally, nobody currently use nsersult.
So, we can get rid of it now.
Fortunately, nobody including comm-central and BlueGriffon implements
WillSplitNode(). So, we can get rid of it. However, removing interface
method should be done in a follow up bug. So, we can remove offset computation
in EditorBase::SplitNode() completely in the future.
MozReview-Commit-ID: JWj34SjBNJh
--HG--
extra : rebase_source : f0e1ed0e466dc8217c1a0ab1722790883a7efd1f
EditorBase::CreateTxnForSplitNode() and EditorBase::SplitNode() takes a set of
container and offset in it for specifying a point to split.
Instead, they should take EditorRawDOMPoint for specifying start of right node.
MozReview-Commit-ID: 5VwS8oudzIT
--HG--
extra : rebase_source : 727948e5cf95f0713019f57ae9a007b85569fa56
Make SplitNodeTransaction stores start of existing right node (which will be
split) instead of split point as a pair of the right node and offset in it.
MozReview-Commit-ID: 2DIpJGSuNaC
--HG--
extra : rebase_source : 13949bdddc30c59462e7fea7fadf29f015ab8d3a
EditorBaseSplitNodeImpl() should be clean up with EditorDOMPoint which should
be an argument to point the first point of right node (existing split node).
MozReview-Commit-ID: DN0yHm9G9yT
--HG--
extra : rebase_source : 256b4e2125e831b7be9e5c4aefc6f04c80e3c1f5
We hit assertion which were added by bug 1414713. That tells us an ancient bug.
There is comment which claims that we should move all list items in the right
list node to left list node if the list nodes should be merged. However, this
has never been done actually since 2002. The code tried to move *some* list
items in the right list node to the list list node. However, retrieving first
list item at an offset almost always failed because the offset variable has
never been initialized.
If we believe the comment, we should move all children of the right list node
to the left list node. However, until we get a testcase to reach this case,
we should keep current behavior, i.e., do nothing, for unexpected regression.
MozReview-Commit-ID: 1r81q1m44oW
--HG--
extra : rebase_source : fc02520f76440d5900fec0517aa6bcd188e1a14f
m-c, c-c and bluegriffon don't use this interface. So we should remove this.
MozReview-Commit-ID: 92VwGKlrOw5
--HG--
extra : rebase_source : 003763c6abc1406bcffabbffc3fe0696985ee97a
No one uses out parameter of InsertBR, so we should remove it. Also, CreateBR
has direction parameter for selection, so we should use it.
MozReview-Commit-ID: 8heqaXpR9He
--HG--
extra : rebase_source : 27b952f080a70fe039c9b43d7ae9f1394d4de01e
ReturnInListItem and ReturnInHeader uses Element as parameter, but
ReturnInParagraph doesn't use Element parameter even if it is Element.
So we should use Element for parameter.
MozReview-Commit-ID: 35JJTETFK45
--HG--
extra : rebase_source : aa101c921d176b7ae7807c461bca1c4a51f9aecc
Now, we know HTMLEditRules::ReturnInParagraph() always splits the parent block
at start of selection. So, it doesn't need to receive the position from the
caller because the cost to get start of selection from first range of Selection
is really cheap.
MozReview-Commit-ID: EvNb6lUBLdt
--HG--
extra : rebase_source : 448fcb4e3a3c6df777825e8747839d6cd9ee2d56
The only caller of HTMLEditRules::ReturnInParagraph() is
HTMLEditRules::WillInsertBreak(). It should use EditorDOMPoint for storing
selection start.
Then, this patch makes what is set to ReturnInParagraph() clear. So, the
point ReturnInParagraph() is always start of selection.
MozReview-Commit-ID: 6X0P5gpwXKr
--HG--
extra : rebase_source : d1814e5b7c8e4005886607691b37fe6a83e8e5be
|selNode| and |selOffset| in HTMLEditRules::ReturnInParagraph() store the
point to split |aParentDivOrP|. So, they should be |containerAtSplitPoint|
and |offsetAtSplitPoint|.
Then, |newSelNode| can be renamed to |splitAfterNewBR|.
MozReview-Commit-ID: 1DcmLNx1Cff
--HG--
extra : rebase_source : 38e7e1ec68c060da3ea31f694243adc004eeece7
HTMLEditRules::ReturnInParagraph() stores point to insert new <br> element with
a set of |parent|, |node| and |offset|. Their names are really unclear and
they're really messy.
So, let's use new local variable, |EditorRawDOMPoint pointToInsertBR|. This is
set only when new <br> element is necessary. So, we can get rid of
|newBRneeded| too.
MozReview-Commit-ID: EdnhnlokurN
--HG--
extra : rebase_source : d97a985d71a7b4552ccdeedde55f8b7fcb0be590
This cleans up |aSelection| and |aPara| of HTMLEditRules::ReturnInParagraph().
|aSelection| should be reference for avoiding nullptr check.
|aPara| is so too. Additionally, the name is not clear. We should rename it
to |aParentDivOrP| because it's a block parent of the point and has to be
<div> or <p> element.
MozReview-Commit-ID: 8LbKGlrvaIj
--HG--
extra : rebase_source : e0593c916791ec5b39b4138e21b6ce8c680dc4d8
The local variable |sibling| in HTMLEditRules::ReturnInParagraph() is set to
aBRNode of SplitParagraph(). So, the name should be renamed to |brNode|.
Then, the |brNode| should be cleared if it's not a <br> node actually. However,
SplitParagraph() doesn't allow aBRNode to be nullptr. aBRNode is only used to
remove it from the document if it's not necessary. So, it should be able to be
nullptr. Therefore, this patch changes SplitParagraph() too.
Note that SplitParagrah() is called only by ReturnInParagraph(). So, we don't
need to worry about other callers.
MozReview-Commit-ID: 7Ynk9m5F8Mi
--HG--
extra : rebase_source : e2583c70ad274fe74f48df687796ed71a66bdf98
For reducing the number of arguments of HTMLEditRules::ReturnInParagraph(),
let's make it return EditActionResult and get rid of |aCancel| and |aHandled|.
MozReview-Commit-ID: HU1SthEjonn
--HG--
extra : rebase_source : 072865996aaccc63924c34f91014b41b64aa1a16
EditorBase::CreateNode() should take EditorRawDOMPoint as insertion point of
the new element instead of a set of container, child and offset of the child
in the container.
This patch initializes EditorRawDOMPoint with original 3 arguments as far as
possible. If the relation of them are broken, MOZ_ASSERT in RawRangeBoundary
constructor detects existing bugs.
MozReview-Commit-ID: 2N55S6pRv7k
--HG--
extra : rebase_source : 2b14a7715815ca0007635b8f791ca9edbe5b65f1
nsIEditActionListener::WillCreateElement() and
nsIEditActionListener::DidCreateElement() are implemented only by m-c.
So, we can remove a set of container node and offset in it from their argument.
Instead, WillCreateElement() should take a node which will be next sibling of
the new node.
Note that only implementation of them is, HTMLEditRules::DidCreateElement().
So, we can get rid of them and can call HTMLEditRules::DidCreateElement()
directly from EditorBase::CreateNode(). However, such change should be done
in another bug which checks all nsIEditActionListener method implementations.
MozReview-Commit-ID: 4LQEs2WwrVC
--HG--
extra : rebase_source : ee1bee1413c578b2873a291c712b8ef46221db0f
The constructor of CreateElementTransaction now takes EditorRawDOMPoint instead
of a set of container node and offset in it. So, its only user,
EditorBase::CreateTxnForCreateElement(), should take EditorRawDOMPoint too.
MozReview-Commit-ID: A8QfPM3LRii
--HG--
extra : rebase_source : 4a99e5cb58230649d19faca788a330fe02eb6bb1
The constructor of CreateElementTransaction should use EditorRawDOMPoint to
receive insertion point of the new element. Then, it should be stored with
RangeBoundary. With this change, CreateElementTransaction doesn't need to
compute child node at insertion point.
Additionally, this creates InsertNode() method. Current code works differently
when DoTransaction() is called and RedoTransaction() is called.
MozReview-Commit-ID: 8ujhmzn65Wg
--HG--
extra : rebase_source : 30cc045a30a3836f211d11e5f70a85804f44a72a
By the fix of bug 1407352, DeleteRangeTransaction::CreateTxnsToDeleteBetween()
starts to use child node instead of a set of container node and offset in it.
However, this is ugly and using RawRangeBoundary may reduce computing offset
more. So, they should use RawRangeBoundary to refer delete points.
MozReview-Commit-ID: pwDHOxpz0E
--HG--
extra : rebase_source : 3871ba4fa3cfb174d5dac57822ae69c40a305323
In some places, editor computes index from child node for collapsing selection
at the child node. However, it's expensive. Therefore, editor should use
Selection::Collapse(const RawRangeBoundary&) as far as possible.
MozReview-Commit-ID: LF2MwASuXzZ
--HG--
extra : rebase_source : b7afc35c0d9d88845391b6f18de57cbff1935ae4
Bug 1402904 added nullptr check for parent node, but I forgot to add this
nullptr check of selected node for outdent command.
So I need to add more nullptr check.
MozReview-Commit-ID: Au9wrG6htk8
--HG--
extra : rebase_source : bd73b1b667b6dc869ebcea3fdfbe1eb3b04a8cde
