Additionally, this patch renames its specific enum class from ContentsOnly
to ResetAlignOf for making its target clearer.
MozReview-Commit-ID: KD4ndAsMClN
--HG--
extra : rebase_source : 302598397cf32893c7db18a6a014d0db862cb8f6
Additionally, this patch renames it to ChangeMarginStart() for making its
job clearer and add two inline wrapper methods.
MozReview-Commit-ID: L2GfLKhT6sa
--HG--
extra : rebase_source : d1d293f742214048ce5dfbbb7ebd50b0cbeed881
Some Did*() of TextEditRules and HTMLEditRules do nothing except returning
NS_OK. Let's remove them since there is no reason to keep them.
MozReview-Commit-ID: Jcdh5rnm5Yc
--HG--
extra : rebase_source : 54aedd8187edfa244bc8fbe8f560fb6ee064e9cf
Now, each protected/private method of TextEditRules and HTMLEditRules can
retrieve Selection instance safely and quickly. Therefore, their pointer or
reference of Selection arguments are not necessary. Therefore, this patch
removes them.
MozReview-Commit-ID: 1SPmKXmd7kz
--HG--
extra : rebase_source : 32dfd2d5e8b0667318a5b3766a8a7d8594bba8e7
Except HTMLEditRules::WillJoinNodes(), observer methods of HTMLEditRules
accesses mHTMLEditor. Therefore, we need to make them create AutoSafeEditorData
instance in the stack.
Note that for reducing EditorBase::GetSelection() calls, this patch adds
Selection& argument to all of them.
MozReview-Commit-ID: 6mjuD2gZwVp
--HG--
extra : rebase_source : 56f16747a80927f0477b9b54f6088be719ed7b01
This patch creates a stack class in TextEditRules, its name is
AutoSafeEditorData and make all public methods (except editor observer methods)
which may change DOM tree, fire some DOM events or calling some XPCOM listeners
create its instance in the stack. Then, it grabs associated editor instance
and its selection instance.
Next patch will make remaining public methods create AutoSafeEditorData.
MozReview-Commit-ID: 8oshdhL3ONQ
--HG--
extra : rebase_source : 591db71e45fe28ca93cbebd9bb7da8c16eae4466
This patch renames:
EditorBase::SplitNode() -> EditorBase::SplitNodeWithTransaction()
EditorBase::SplitNodeDeep() -> EditorBase::SplitNodeDeepWithTransaction()
HTMLEditRules::MaybeSplitAncestorsForInsert() ->
HTMLEditRules::MaybeSplitAncestorsForInsertWithTransaction()
Note it might be that some callers of those methods should be renamed too.
However, we should do it in follow up bug after landing those patches since
we can investigate it with searchfox.org after landing patches.
MozReview-Commit-ID: FfxCfaI85z5
--HG--
extra : rebase_source : 143960de45791c72dbf4d436f65e55452b4f7b10
To reduce QI, I would like to replace nsIDOMNode with nsINode. And some
parameters in *DataTransder.cpp's methods is unused (it uses as nullptr),
so we should remove these parameters.
Also nsIDOMNodeList is unused now, so we should remove this including.
MozReview-Commit-ID: 1QTIkxDazjJ
--HG--
extra : rebase_source : 3961e897fcaa96975facc822821f1e223cab358d
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
Except table access and XPCOM methods for c-c, tests and etc, we can remove
more nsIDOMElement usages to avoid QI.
MozReview-Commit-ID: HO5kAaZAs6Q
--HG--
extra : rebase_source : 41ede0bace33504ad852dc4e0016ea346cd7bdee
HTMLEditRules::WillInsertBreak() started to use HTMLEditRules::MakeBasicBlock()
to wrap existing inline elements with default paragraph separator if inline
elements are children of editing host. However,
HTMLEditRules::ApplyBlockStyle() called by HTMLEditRules::MakeBasicBlock() sets
mNewNode to last new block node. So, if it wraps inline elements after caret
position, mNewNode becomes after expected caret position and
HTMLEditRules::AfterEditInner() will move caret into it.
This patch make HTMLEditRules::WillInsertBreak() reset mNewNode with
caret position after calling HTMLEditRules::MakeBasicBlock().
Additionally, this patch fixes a bug of HTMLEditor::IsVisibleBRElement().
That is, it uses only editable nodes to check if given <br> element is
visible. However, editable state is not related to this check. If <br>
element is followed by non-editable inline node (except invisible data
nodes), it always visible. This bug caused getting wrong nodes with
HTMLEditRules::GetNodesFromSelection() which is used by
HTMLEditRules::MakeBasicBlock(). Therefore, this patch also adds lots of
EditorBase::Get(Next|Previous)ElementOrText*() and
HTMLEditor::Get(Next|Previous)HTMLElementOrText*() to ignore only invisible
data nodes.
Note that even with this fix, the range of nodes computed by
HTMLEditRules::GetNodesFromSelection() is still odd if only non-editable
elements follow a <br> node which is first <br> element after the caret
position. However, what is expected by the execCommand spec is unclear.
So, automated test added by this patch checks current inconsistent behavior
for now.
MozReview-Commit-ID: 2m52StwoEEH
--HG--
extra : rebase_source : 6b9b2338e35c4d2e89a405fd8e1ffc7b0873ca1e
HTMLEditRules implements only some of nsIEditActionListener and this is always
first edit action listener. So, if we make EditorBase treat HTMLEditRules
directly before notifying edit action listeners, we can save a lot of runtime
cost (virtual calls especially unnecessary, copying array of edit action
listeners with strong pointer, redundant QIs), although the code becomes not
beautiful.
Perhaps, we should do same thing to nsTextServicesDocument and
mozInlineSpellChecker in other bugs.
MozReview-Commit-ID: Eveaxj398f2
--HG--
extra : rebase_source : 0b7b66ba1002e08591e8d95ef68b216e7ce1f93b
For calling some methods of mRules from EditorBase, let's move mRules member
from TextEditor to EditorBase.
Unfortunately, TextEditRules.h depends on EditAction which is declared in
EditorBase.h and that caused unnecessary include hell of EditorBase.h. So,
let's move it to an independent header file.
MozReview-Commit-ID: 5HiSZLP9WHH
--HG--
extra : rebase_source : 3e2c40385a6f3d6d1e03ef4e213434383bb37d5f
Actually, PopItem uses nsString to store attribute name. And when using it,
it converts to nsAtom by NS_Atomize and compare by nsString.
To reduce calculation of atom and string compare, we should store atom directly.
MozReview-Commit-ID: 8OB02mgMg1r
--HG--
extra : rebase_source : 9861217804acac52106a89cad75e08bd547080ad
extra : histedit_source : 2b9f01cf29b1b09dd07fc771ff94a06be4d94f31
nsIEditRules is a super class of only mozilla::TextEditRules and not scriptable.
So, we can get rid of it.
This patch merges RulesInfo with TextRulesInfo and name new class is RulesInfo
for minimizing the code change.
Additionally, adds two methods AsHTMLEditRules() and its const version.
They make existing cast code safer.
MozReview-Commit-ID: KwWH3ADj3Bv
--HG--
extra : rebase_source : 4517bdc95b530530e9756e07c4b6cce78c002073
HTMLEditRules::IsEmptyBlock() won't return error in most cases. Additionally,
HTMLEditRules::WillInsertBreak() doesn't check it. So, just returning bool
is simpler.
MozReview-Commit-ID: 5DfRv7lIyuS
--HG--
extra : rebase_source : 8b430d88a92fd5830a0b9f1bc1d46ac31e45c12c
HTMLEditRules::StandardBreakImpl() should take |const EditorDOMPoint&| to
specify where a new <br> element inserted before.
Additionally, we should rename it to InsertBRElement().
MozReview-Commit-ID: 4BR7xGFFrpk
--HG--
extra : rebase_source : 873af2c4167227a570878fe03d6dec61dffe9528
Although nsIEditActionListener::WillInsertNode() nobody implements actually,
we should remove it in a follow up bug.
nsIEditActionListener::DidInsertNode() is implemented only by HTMLEditRules.
So, if we make it not use nsIEditActionListener, we can remove it too.
However, keep it for now.
On the other hand, they don't need to receive index of the insertion point.
WillInsertNode() needs next sibling of the insert point, but DidInsertNode()
needs nothing because listener can compute it with new inserted node.
MozReview-Commit-ID: GiTKkVyZJlN
--HG--
extra : rebase_source : 9ee38c28217d25d1a3f79b0b458c7b2121350a76
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
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
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
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
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
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
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
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
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
(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
--HG--
rename : xpcom/ds/nsIAtom.h => xpcom/ds/nsAtom.h
extra : rebase_source : ac3e904a21b8b48e74534fff964f1623ee937c67
This is a typo bug of Bug 1053779 Part 2. ConvertListType might return NS_OK
even if ReplaceContainer doesn't return valid value.
So, to clean up code, we should return Element instead of nsresult since out
parameter of this function is Element only.
MozReview-Commit-ID: 44UHETzcdGy
--HG--
extra : rebase_source : ab76486505cb4e7caea3fb99e11fccd606878f02
