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
nsITextServicesDocument isn't scriptable, so there is no reason to use
nsIDOMRange.
MozReview-Commit-ID: AVPgrwmz38H
--HG--
extra : rebase_source : 0cb03cdb9b0dc4ef85b1b5582b7e3a54cb002904
nsTextServicesDocument still use nsIDOMNode, so we should replace with nsINode
to reduce QI.
MozReview-Commit-ID: 7G9w31dRFi9
--HG--
extra : rebase_source : 322ae89f42183fe780b408f2ceb1c6d699ade985
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
libpref only allows pref modifications in the parent process. This patch
tightens up the checking, which is a bit inconsistent.
- It removes ENSURE_MAIN_PROCESS_WITH_WARNING, which does NS_WARNING on
failure, and replaces its uses with ENSURE_MAIN_PROCESS, which does NS_ERROR
on failure. This required adding an XRE_IsParentProcess() check to one place
in editor/.
- It converts XRE_IsContentProcess() tests to !XRE_IsParentProcess(), because
we now have multiple kinds of non-parent process.
- It uses ENSURE_MAIN_PROCESS to replace other checking code in a few places.
- It improves a comment in HandleDirty().
MozReview-Commit-ID: D8znQWH7ery
--HG--
extra : rebase_source : ea0fc095b31525bde82a1be217923512d030b76d
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
Masayuki Nakano
Makoto Kato
Brindusan Cristian
Olli Pettay
Edgar Chen
Emilio Cobos Álvarez
Bogdan Tara
Nicholas Nethercote
Ryan VanderMeulen
Kyle Machulis