The name, WSRunObject::CheckTrailingNBSP(), is unclear what it does. It
replaces previous character of specified point with ASCII space if it is NBSP
and it's not necessary.
So, it should be renamed to ReplacePreviousNBSPIfUnncessary().
Additionally, it should take |const EditorRawDOMPoint&| instead of a set of
container node and offset in it.
MozReview-Commit-ID: 2vq46OiAzo6
--HG--
extra : rebase_source : 132bb71361ff1e60268863cd4ef86bfbd0ddc9d1
WSRunObject::GetChar(After|Before)(nsINode*, int32_t) returns next/previous
character's DOM point as WSPoint. If the container node is a text node in
mNodeArray, it calls WSRunObject::GetChar(After|Before)(const WSPoint&) which
returns next/previous offset if the point isn't end/start of the text node.
If the point is at end/start of the text node, it returns start/end of
next/previous text node in mNodeArray. If the container node is not a text
node in mNodeArray, it calls WSRunObject::GetWSPoint(After|Before)(). It looks
for next/previous text node in mNodeArray from the point. Then, it calls
WSRunObject::GetChar(After|Before)(const WSPoint&) and returns the result.
So, we should rename GetCharAfter() to GetNextCharPoint(), GetCharBefore() to
GetPreviousCharPoint(), GetWSPointAfter() to GetNextCharPointInternal() and
GetWSPointBefore() to GetPreviousCharPointInternal().
Then, they should take |const EditorRawDOMPoint&| instead of a set of container
node and offset in it.
So, looks like that "WS"RunObject is not good name for this class, perhaps,
AutoTextRunManager or something? But I'm still not sure.
MozReview-Commit-ID: 85cX3MdlFwz
--HG--
extra : rebase_source : 217dbb75653bf2a4c593fb9a0498824b1b764cf4
WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments
to represent a range to remove.
Additionally, this renames it to DeleteRange() because it also removes any
nodes in the range. So, "Chars" isn't good word for this method's name.
MozReview-Commit-ID: 5Dmxnia1JPO
--HG--
extra : rebase_source : a4fd44c9354762db0e37fa156790dc20fdbe23a1
WSRunObject::FindRun() finds the nearest run from aPoint to specified direction.
So, it uses nsContentUtils::ComparePoints() a lot. Therefore, it should use
an overload which takes RawRangeBoundary. Although, it's not optimized for
RawRangeBoundary, but if it'd be optimized, this method becomes faster.
And this patch renames it to FindNearestRun().
MozReview-Commit-ID: 2NkR5E1st6d
--HG--
extra : rebase_source : 387ecc9d483c7cd88306197391fc2940b2000e28
WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| instead of
a pair of container node and offset in it.
MozReview-Commit-ID: 38OAn4dvR7x
--HG--
extra : rebase_source : 8e48ba2cec07c1dd4daf3cd9ec44751c860bc3da
TextEditor::CreateBRImpl() should take |const EditorRawDOMPoint&| as insertion
point of new <br> element. Additionally, it doesn't need to have out argument
for the point of after <br> element because callers can get it with
EditorRawDOMPoint(nsINode*) and the new <br> node, and calling AdvanceOffset().
This cost must be enough cheap.
MozReview-Commit-ID: Hxawz3D2dCd
--HG--
extra : rebase_source : 866ff50efd70499ed733da9efcce7399f44bd4a0
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
EditorUtils::IsDescendantOf() current takes a pointer to offset or a pointer to
child content if the caller needs to know the child of the most ancestor node.
However, some callers should get a child as EditorDOMPoint or EditorRawDOMPoint.
Then, they can be used for some editor methods which need to take child node
for performance optimization.
This patch makes EditorUtils::IsDescendantOf() as only two overloads. One takes
pointer to EditorRawDOMPoint as optional out argument. The other takes pointer
to EditorDOMPoint as an out param.
Additionally, this creates new constructor of AutoTrackDOMPoint for making it
can treat EditorDOMPoint directly.
MozReview-Commit-ID: IsAGTUvKI19
--HG--
extra : rebase_source : 97469a21b974c6a1dd515ab472bbc4a88c1899c8
RangeBoundaryBase shouldn't compute mRef when it's initialized with offset.
E.g., some users of the template class may initialize it with a container and
offset in it but it may not refer mRef or may refer after modifying offset.
On the other hand, RangeBoundaryBase::InvalidateOffset() is a special method.
It assumes that mRef is always initialized when it's called but can be
invalidate mOffset until retrieved actually. This is necessary for
nsRange::mStart and nsRange::mEnd since the offset may be changed when
some nodes are inserted before the referring node.
So, now, InvalidateOffset() should be a protected method and make nsRange a
friend class of RangeBoundaryBase. Then, when nsRange sets mStart and/or mEnd,
nsRange itself should guarantee that their mRefs are initialized.
MozReview-Commit-ID: Alr4YkDXIND
--HG--
extra : rebase_source : 7e6828374db7989ae91b9e485571ec553f7435af
A lot of methods in editor returns a child offset with an out param when it
returns its container and offset in the container. This is ugly hack for
performance of nsINode::IndexOf(). However, there are a lot of regression
since the relation between offset and child node can be broken really easily.
So, we should make EditorDOMPoint as a subclass of RangeBoundary and manage
a set of container, reference child and its offset in it (e.g.,
SetNextSibling() added by this patch).
Note that RangeBoundary's performance is not good for temporary use if we set
a point with offset, it immediately retrieves mRef. The following patch will
improve this performance.
MozReview-Commit-ID: 7mcJ1P1OjVr
--HG--
rename : editor/libeditor/EditorUtils.h => editor/libeditor/EditorDOMPoint.h
extra : rebase_source : 785094fcfc592d9e5b48cbc36ed225dbb8bb4111
HTMLEditRules::BustUpInlinesAtRangeEndpoints() tries to split all inline nodes
at range start and range end. It uses EditorBase::SplitNodeDeep() to split
the nodes and HTMLEditRules::GetHighestInlineParent() to retrieve the highest
inline parent of them.
Currently, HTMLEditRules::GetHighestInlineParent() may return editing host or
ancestor of it if active editing host is not block. Then, it may cause
splitting editing host or its parents and following methods of HTMLEditRules
will fail to modify the nodes created outside the editing host.
So, HTMLEditRules::GetHighestInlineParent() should return only one of the
descendants of active editing host.
Unfortunately, even if just adding the test case as a crash test, I cannot
reproduce the crash with automated tests. Therefore, this patch doesn't
include any automated tests.
And this patch changes a crash test, 1402196.html, which expects that an inline
editing host is split by execCommand("insertOrderedList"). However, this patch
fixes this wrong behavior. Therefore, this patch changes the event target of
event listener from <p> inside the editing host to the editing host itself.
MozReview-Commit-ID: 8i5ci1fcrDd
--HG--
extra : rebase_source : 572a7b22550a38ca71c954f62eefa695addd53c2
And remove unreachable code after MOZ_CRASH().
MozReview-Commit-ID: 6ShBtPRKYlF
--HG--
extra : rebase_source : 0fe45a59411bda663828336e2686707b550144ae
extra : source : 8473fd7333d2abe1ea1cc176510c292a5b34df45
Currently, HTMLEditRules::WillInsertBreak() checks if the editing host can
contain a <p> element as a child or a descendant. However, this is not enough.
If an inline element has a block element which can contain a <p> element,
current implementation considers to insert a <br>. This is possible when
* The editing host is an unknown element including user defined element.
* The editing host is an inline element and its children and/or descendants
were added by JS. E.g., <span contenteditable> element can have <div>
element.
I think that we should consider to insert a <br> element when:
- There is no block ancestors in the editing host.
- The editing host is the only block element and it cannot contain <p> element
or the default paragraph separator is <br> element.
- The nearest block ancestor isn't a single-line container declared in the
execCommand spec and there are no block elements which can contain <p>
element.
Note that Chromium checks if CSS box of ancestors is block too. However,
it must be out of scope of this bug.
MozReview-Commit-ID: HdjU9t83Nd1
--HG--
extra : rebase_source : 7030671268a610613359b5d8ae5d126e120f59bd
Perhaps, most callers don't need parent block outside active editing host.
Therefore, callers of these methods should be able to specify the editing
host for making those methods stop looking for a block ancestor.
Then, callers can avoid using EditorUtils::IsDescendantOf() and
nsContentUtils::IsContentDescendantOf().
MozReview-Commit-ID: 7IK4gAVHY5d
--HG--
extra : rebase_source : 31cd55026f0ce005d906499de4ebe5d1c39555e9
Currently, HTMLEditor::GetBlockNodeParent(nsIDOMNode*) is used only by
HTMLEditor::DoInsertHTMLWithContext() and there is a variable of nsINode*.
So, we don't need to keep it anymore.
MozReview-Commit-ID: LEWaiR5BEB9
--HG--
extra : rebase_source : 64ef772f3b7883bd4aae48dec737663e6036553b
SetAttributeOrEquivalent can remove CSS property that is same behaviour by
part 2. So we should use it instead of SetAttribute.
MozReview-Commit-ID: InCZQVdbDRm
--HG--
extra : rebase_source : d746b07bcfffc752ad90b9e1504e2f0afc23d457
SetAttributeOrEquivalent sets element's attribute when
HTMLEditor::IsCSSEnabled() is false. It should remove the CSS property that is
same behaviour too.
MozReview-Commit-ID: ChKjlB7wI0Z
--HG--
extra : rebase_source : c31b940394750757b2393a6876534590410d9398
Actually, we don't consider CSS property to get alignment when IsCSSEnabled()
is false. For WebKit and Blink compatibility, we should consider this situation.
MozReview-Commit-ID: 9ORntUmbIbf
--HG--
extra : rebase_source : 21d27f34cf1331bd2fee097c5c445dc16c453d4e
WSRunObject::InsertText() may delete given child node at offset with calling
DeleteChars() or CheckLeadingNBSP() before calling HTMLEditor::InsertTextImpl().
Therefore, even though using nsINode::GetChildAt() is slow, it needs to update
aInOutChildAtOffset after calling them.
MozReview-Commit-ID: AbTTfNAjMIK
--HG--
extra : rebase_source : b4282e8dc6e395acc89d7c155bfeae46e7c41e4a
The bug is a report for hitting MOZ_ASSERT, but we should also check the result
of the call of execCommand.
MozReview-Commit-ID: FydZKAjI2Rl
--HG--
extra : rebase_source : 327988cea366474cbe659a3b80c09ce7ceef0005
RangeItem::GetRange can return null, so we shouldn't add null range to
aArrayOfRanges.
MozReview-Commit-ID: Ci5VHeqE8km
--HG--
extra : rebase_source : 83abb5f4ea405c360e63b7ed73530a7321a2a1e5
RangeItem::GetRange can return null, so we shouldn't add null range to
aArrayOfRanges.
MozReview-Commit-ID: Ci5VHeqE8km
--HG--
extra : rebase_source : 34f5e98de680ffe588d64244592f775450ce98cf
This crash test can cause crash before landing the patches for bug 1415509.
So, let's take this for regression test.
MozReview-Commit-ID: 652wi49e720
--HG--
extra : rebase_source : a7a897670220e4f14df91c4093e65aa7da7f6015
First, the method name is not correct. It tries to find an editable node near
the given DOM point. Therefore, it should be FindNearEditableNode().
Next, the implementation did something odd. E.g., in the first |if| block,
when |nearNode| is nullptr, it returns nullptr. However, following |if| block
does something only when |nearNode| is nullptr. So, we can get rid of the
second |if| block. Then, nobody will change aDirection. So, we can make it
not a reference now.
Similarly, in |while| block, if |nearNode| becomes nullptr, it returns error.
However, following block checks if |nearNode| is NOT nullptr. So, we can get
rid of this |if| statement and outdent its block.
Additionally, |curNode| isn't necessary. It only increments the refcount
redundantly. So, we can get rid of it.
Finally, FindNearEditableNode() can return found node directly instead of
error code because error code doesn't make sense. Not found an editable
node is not illegal. And also it can take EditorRawDOMPoint instead of
a set of container, child and offset of the child in the container.
MozReview-Commit-ID: CTI581PhJMd
--HG--
extra : rebase_source : 7e05998721ce96727d40dda1be5e7e36b090bcd3
HTMLEditor::GetNextHTMLNode() should be redesigned as
HTMLEditor::GetNextEditableHTMLNode(nsINode&),
HTMLEditor::GetNextEditableHTMLNodeInBlock(nsINode&),
HTMLEditor::GetNextEditableHTMLNode(const EditorRawDOMPoint&) and
HTMLEditor::GetNextEditableHTMLNodeInBlock(const EditorRawDOMPoint&).
Same as GetPreviousEditableHTMLNode*(), we don't need the methods to find
non-editable nodes too.
MozReview-Commit-ID: JjZauCMblp4
--HG--
extra : rebase_source : 1e650d79bc21d7f920b3f46f4a2f208ac23cc0a6
HTMLEditor::GetPriorHTMLNode() methods are similar to EditorBase::GetPriorNode()
which was redesigned with the previous patch.
So, it should be redesigned as
HTMLEditor::GetPreviousEditableHTMLNode(nsINode&),
HTMLEditor::GetPreviousEditableHTMLNode(const EditorRawDOMPoint&),
HTMLEditor::GetPreviousEditableHTMLNodeInBlock(nsINode&) and
HTMLEditor::GetPreviousEditableHTMLNodeInBlock(const EditorRawDOMPoint&).
Note that HTMLEditor::GetPriorHTMLNode() are always return editable node.
So, we don't need to create non-editable node methods for them.
Although, I don't like the word "HTMLNode" because this can return SVG element
or something too. The additional feature of those methods is just checking
given node is in active editing host. So, they are for HTML editor, but not
returning only HTML nodes. However, I have no better idea with shorter name.
MozReview-Commit-ID: 3J4IaBOFjzj
--HG--
extra : rebase_source : 712bc8676fcdc37f38fd46083df177c0fe6bd408
An overload of EditorBase::GetNextNode() takes a set of container, child node
and offset of the child in the container. Replacing it with EditorRawDOMPoint
makes the caller simpler.
Additionally, it has two bool arguments, one is for searching if editable node,
the other is for searching if in same block. So, they can be hidden with
some human readable inline methods.
When I was creating this patch, I realized that
GetNextNodeInternal(const EditorRawDOMPoint& aPoint) may return
aPoint.GetChildAtOffset(). I.e., it starts to search from the specified point
rather than next node. On the other hand, GetNextNodeInternal(nsINode& aNode)
never returns aNode itself. So, it there is better name instead of "Next",
we should take it. But I have no better idea. So, this patch just explains
the difference with comments in EditorBase.h.
MozReview-Commit-ID: 4Lb6o9SJuhy
--HG--
extra : rebase_source : d20d728eae69659ef448b6679ae8f73d64c2d7e9
An overload of EditorBase::GetPriorNode() takes a set of container, child node
and offset of the child in the container. Replacing it with EditorRawDOMPoint
makes the caller simpler.
Additionally, it has two bool arguments, one is for searching if editable node,
the other is for searching if in same block. So, they can be hidden with
some human readable inline methods.
Finally, "Prior" isn't a term of DOM. So, let's use "Previous" instead.
MozReview-Commit-ID: A9uKzHaikY9
--HG--
extra : rebase_source : 15bfdfde0ad89a5331d6c8a24351741eeef476d5
Like EditorBase::InsertTextImpl(), WSRunObject::InsertText() is really messy.
So, it should take same arguments as EditorBase::InsertTextImpl().
MozReview-Commit-ID: 5uKGaxKheRv
--HG--
extra : rebase_source : 49ce0eb7ea25b397b6b1a19f1bc21d711740c043
EditorBase::InsertTextImpl() takes |nsCOMPtr<nsINode>*|, |nsCOMPtr<nsIContent>*|
and |int32_t| as in/out arguments for container, child and offset of the child
in the container. But this makes the callers really hard to read and ugly.
So, we should make the method take |const EditorRawDOMPoint&| argument as input
and |EditorRawDOMPoint*| as out argument.
MozReview-Commit-ID: 2ijIfGl4Zo7
--HG--
extra : rebase_source : b309d9bdc04aac620f138769ba18ad7e4597fe6c
EditorBase::FindBetterInsertionPoint() now use 3 in/out arguments. This is
really ugly and making the callers hard to read. So, let's make it take an
argument whose type is |const EditorRawDOMPoint&| and return other
EditorRawDOMPoint instance.
Additionally, this fixes bugs of text node length checks in the method.
Basically, this shouldn't affect to any actual behavior, though. That is
because text node shouldn't be able to have string longer than INT32_MAX.
MozReview-Commit-ID: FClUQSJzd8c
--HG--
extra : rebase_source : 3e2fbd345015f7068c7e35a94c31731e7936009f
Since if the container is not a container node like a text node or the offset
is same as the length, child at the offset can be nullptr.
MozReview-Commit-ID: DCnFwUjNgk7
--HG--
extra : rebase_source : 48823390645d79db391dcaf64dab59e754b680a5
|selChild| should be a node which is at |curOffset| in |curNode|, however,
it's not updated when |curOffset| is updated by a call of CreateBRImpl(). So,
it should be next sibling of the created br node.
MozReview-Commit-ID: IVfTjVMp3lB
--HG--
extra : rebase_source : fbc355e3c0425a495c78c3c028308dac26617283
Editor sometimes extracts atom to string to compare element name.
It is unnecessary to use atom directly.
MozReview-Commit-ID: FEvyiIeaozs
--HG--
extra : rebase_source : 4418d0c82fa4fedd814b914f2cf3a86d74ad9835
Masayuki Nakano
Brindusan Cristian
Olli Pettay
Edgar Chen
Emilio Cobos Álvarez
Makoto Kato
Bogdan Tara
Ryan VanderMeulen
Kyle Machulis
Narcis Beleuzu
Margareta Eliza Balazs
Boris Zbarsky
Sebastian Hengst
Nika Layzell
Chris Peterson
Masatoshi Kimura
Jason Kratzer
Kris Maglione