`WSRunObject` scans previous and next node of given point/range **without**
checking editing host boundary. Therefore, its methods may return non-editable
nodes or editable nodes in another editing host. In such cases, `HTMLEditRules`
is confused.
This patch makes it store editing host at initialization and it check the
boundary. However, the former cost may appear in score of some benchmark
tests, but we shouldn't allow attackers to use this entrance.
Differential Revision: https://phabricator.services.mozilla.com/D32467
--HG--
extra : moz-landing-system : lando
It'd be better to change copy constructor of `EditorDOMPointBase` to explicit,
but it'd require too many changes in editor code. So, this patch just changes
each method callers only.
Differential Revision: https://phabricator.services.mozilla.com/D30054
--HG--
extra : moz-landing-system : lando
If insertion string ends with ASCII whitespace and there is no following
content in the block, `HTMLEditRules::AdjustWhitespaces()` needs to insert
`<br>` element. It's called only by `HTMLEditRules::AfterEditInner()` and
that does only simple things with `WSRunObject`. Therefore, this moves the
code into `AfterEditInner()`.
For making it adjust the whitespaces, `HTMLEditRules::WillInsertText()` needs
to notify `AfterEditInner()` of dirty range with `mDocChangeRange`. Therefore,
this patch makes it set `mDocChangeRange` manually after inserting composition
string.
On the other hand, there is another bug. `WSRunObject` was designed to treat
only inserting text for `WSRunObject::InsertText()`. I.e., not designed to
treat replacing existing composition string with new string. Therefore,
`WSRunObject::InsertText()` adjusts whitespaces only around start of
composition string. Therefore, if composition string ends with an ASCII
whitespace, it's not replaced with NBSP and that causes:
- failing `WSRunObject::AdjustWhitespaces()` inserts `<br>` element at
`AfterEditInner()` of committing composition.
- then, next composition's first `WSRunObject::InsertText()` removes the
last whitespace due to not followed by `<br>` nor any other content.
Therefore, this patch makes `WSRunObject` takes 2 DOM points to be able to
treat replaced range.
In strictly speaking, the latter change require more changes and tests for
supporting replacement with any other methods. However, it's risky and out
of scope of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D26423
--HG--
extra : moz-landing-system : lando
This patch marks `EditorBase::InsertNodeTransaction()` **and** its callers as `MOZ_CAN_RUN_SCRIPT`.
Unfortunately, this patch tells us that some `GetSomething()` methods may destroy the editor since `HTMLEditRules::GetNodesForOperation()`, `HTMLEditRules::GetNodesFromPoint()` and `HTMLEditRules::GetNodesFromSelection()` may change the DOM tree. Additionally, initialization methods may destroy the editor since it may insert a bogus `<br>` node.
Note that this patch also removes some unused methods. I.e., they are not result of some cleaning up the code. This patch just avoids marking unused methods as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D25027
--HG--
extra : moz-landing-system : lando
This patch marks `EditorBase::InsertNodeTransaction()` **and** its callers as `MOZ_CAN_RUN_SCRIPT`.
Unfortunately, this patch tells us that some `GetSomething()` methods may destroy the editor since `HTMLEditRules::GetNodesForOperation()`, `HTMLEditRules::GetNodesFromPoint()` and `HTMLEditRules::GetNodesFromSelection()` may change the DOM tree. Additionally, initialization methods may destroy the editor since it may insert a bogus `<br>` node.
Note that this patch also removes some unused methods. I.e., they are not result of some cleaning up the code. This patch just avoids marking unused methods as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D25027
--HG--
extra : moz-landing-system : lando
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.
Overall it's not a very interesting patch I think.
nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.
I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.
While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
AutoTransactionBatch, AutoPlaceholderBatch, AutoSelectionRestorer,
AutoTopLevelEditSubActionNotifier, AutoTransactionsConserveSelection and
AutoUpdateViewBatch access protected members of EditorBase. The access
scope management assume that they are used only by EditorBase or its
subclasses or TextEditRules or its inherited class (i.e., HTMLEditRules).
For guaranteeing this at build-time, we should change them to nested classes
of EditorBase. Then, EditorBase and its subclasses and friends can use them.
Differential Revision: https://phabricator.services.mozilla.com/D9479
--HG--
extra : moz-landing-system : lando
Summary:
Sometimes the caller of PriorVisibleNode or NextVisibleNode doesn't use
outVisNode and/or outvisOffset. But both methods require all parameters and
it don't allow nullptr. It is complex whether parameter is used or unused.
So I would like to allow nullptr.
Also, this methods can change to const method, so I will change some methods
to const method too for this change.
Reviewers: masayuki
Tags: #secure-revision
Bug #: 1483434
Differential Revision: https://phabricator.services.mozilla.com/D3387
--HG--
extra : rebase_source : 5bd9c87f05c8e88879268188a46a4a80126bd3e5
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
Similar to the constructor of WSRunObject, PriorVisibleNode() and
NextVisibleNode() callers may become ugly if the callers start to use
Editor(Raw)DOMPoint. So, let's make them take Editor(Raw)DOMPoint instead of
a pair of nsINode* and offset in it.
Note that a lot of callers of them already use Editor(Raw)DOMPoint. So, we
don't need to keep maintain overloads which takes nsINode* and offset in it
directly.
MozReview-Commit-ID: 3avMtL000nd
--HG--
extra : rebase_source : f8c83e2fabeb5a94a1729c626583a6699636b06c
The following patches make some WSRunObject users use EditorRawDOMPoint or
EditorDOMPoint instead of a pair of nsINode and offset in it. Then, the
code becomes too long like:
> WSRunObject object(mHTMLEditor, pointToDoSomething.GetContainer(),
> pointToDoSomething.Offset());
This is ugly and not easier to read than:
> WSRunObject object(mHTMLEditor, pointToDoSomething);
So, we should create alternative constructor of WSRunObject first.
MozReview-Commit-ID: GiNWRBLl7zB
--HG--
extra : rebase_source : 8616956b9745db130283a4bdbd82155b1cfb2803
WSRunObject::ConvertToNBSP() inserts an NBSP, then, removes following ASCII
whitespaces. When inserting an NBSP, mutation observer may change the
text node. In this case, it shouldn't keep working on removing ASCII
whitespaces because it may causes unexpected result.
This patch also renames ConvertToNBSP() and GetAsciiWSBounds() to
InsertNBSPAndRemoveFollowingASCIIWhitespaces() and
GetASCIIWhitespacesBounds() for making their jobs clearer.
MozReview-Commit-ID: TVy9fEKL6p
--HG--
extra : rebase_source : f0bce124055a86caca57334f06c75a46098f69ac
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
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
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
After bug 769967, FindUserSelectAllNode always returns nullptr. So we get rid of this method. Also, AreaRestriction is used by this, so it is unnecessary now.
MozReview-Commit-ID: HTc8PUCQmy5
--HG--
extra : rebase_source : 11b19a1976f6696e0d872c9020b90701dd22c8c0
Giving '0' (literal zero) to RefPtr is now ambiguous, as both
RefPtr(decltype(nullptr)) and RefPtr(T*) could be used.
In any case, our coding standards mandate the use of 'nullptr' for pointers.
So I'm changing all zeroes into nullptr's where necessary.
MozReview-Commit-ID: A458A4e9for
--HG--
extra : rebase_source : bc4107ce1767329e3dddfe34c55ba79202192e06
Perhaps, there may be better name like WhitespaceRunObject or something, however, for now keep using the term because I don't understand well what it does.
With this patch, following objects are renamed:
nsWSRunObject -> mozilla::WSRunObject
WSType -> mozilla::WSType
nsWSRunObject::WSFragment -> mozilla::WSRunObject::WSFragment
nsWSRunObject::WSPoint -> mozilla::WSRunObject::WSPoint
MozReview-Commit-ID: JgAWiPjOtMW
--HG--
rename : editor/libeditor/nsWSRunObject.cpp => editor/libeditor/WSRunObject.cpp
rename : editor/libeditor/nsWSRunObject.h => editor/libeditor/WSRunObject.h
Masayuki Nakano
Oana Pop Rus
Emilio Cobos Álvarez
Sylvestre Ledru
Makoto Kato
Ehsan Akhgari
Phil Ringnalda
Gerald Squelart
Andrew McCreight