EditorBase::GetStartNodeAndOffset() and EditorBase::GetEndNodeAndOffset() are
just wrappers of EditorBase::GetStartPoint() and EditorBase::GetEndPoint().
They may *compute* offset in the container node even if some callers don't
need the offset. Therefore, we should get rid of them and make all callers
use EditorBase::GetStartPoint() or EditorBase::GetEndPoint() directly.
Note that EditorBase::GetStartNodeAndOffset() and
EditorBase::GetEndNodeAndOffset() returns NS_ERROR_FAILURE if
EditorBase::GetStartPoint() or EditorBase::GetEndPoint() returns not set point.
Therefore, checking the result is set equals checking the old nsresult as
an error.
MozReview-Commit-ID: JLwqRMFLjHC
--HG--
extra : rebase_source : 0eb6bf4ba80e8139c6b9f36723d77d23f2b9099e
It is unnecessary to use already_AddRefed for GetDocument and GetPresShell.
Then, if we remove already_AddRefed, we can replace this with const method.
MozReview-Commit-ID: KTVS0rYrY2i
--HG--
extra : rebase_source : 0199026410fc674f112c960b599a09bc7906cf85
Currently, HTMLEditor doesn't initialize caret position when it gets focus by
itself in most cases. Only when it's in designMode, it may move caret to the
first visible (not checking CSS actually).
In most cases, caret position is adjusted when EditorBase::InitializeSelection()
calls Selection::SetAncestorLimiter(). If selected range is outside of
new limiter, it moves caret to start of the new limiter. However, this is
really different behavior from the other browsers. The other browsers try
to move caret to the first editable text node or before the first editable
content such as <img>, <input>, etc.
This difference causes a serious incompatible issue with Draft.js. It doesn't
initialize caret position when it gets focus but it assumes that caret is
always set to before <br> element if there is no other content.
So, let's try to behave as what other browsers do as far as possible.
This patch makes editor behave as:
* if selection is already in the editing host except start of the editing host,
does nothing.
* if there is non-editable element before any editable node, move caret to
start of the editing host.
* if there is editable text node or element node which cannot have a text node,
move its start or before it.
* if there is no editable nodes which can contain text nodes, move caret to
start of the editing host.
Note that before applying this patch, in designMode, BeginningOfDocument() used
document element instead of <body> element. Therefore, it may set odd position
if <head> element has some text nodes with <script> or <style>. However,
this doesn't make sense and for making more consistent behavior between
designMode and contenteditable, this patch makes it use editing host (it's
<body> element if it's in designMode).
MozReview-Commit-ID: 5neYoTMq6Cc
--HG--
extra : rebase_source : c4d06b6864a221d7cd2833a007d73f7d67821e95
Everyone calls them with the shell of the current composed document, and this
allows the multi-presShell stuff to just be in UpdateCurrentStyleSources /
DoGetStyleContextNoFlush.
The only reason we need to use OwnerDoc()->GetShell() instead of the composed
doc in GetStyleContext / GetStyleContextNoFlush is Element::GetBindingURL, which
does expect to get the binding URL for stuff outside of the composed doc (and
changing that gave me a useless browser).
That's technically a behavior change on the cases that used to pass nullptr, but
I think all callers are fine with that. I could also just add a special function
for that particular case, it may be worth it.
MozReview-Commit-ID: 2XlnkgdgDCK
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
EditorBase::JoinNodeDeep() returns a DOM point which was start of right node.
Currently, this is set before actually joining the nodes. Therefore, we see
warnings since right node which becomes joined node may have less children
than left node.
This patch also makes the NS_WARNING_ASSERTION to NS_ASSERTION since no tests
hit this. So, we can use it to detect regressions.
MozReview-Commit-ID: 3RmRP588AkF
--HG--
extra : rebase_source : 2cab458ffac566c81eb8d3f47e755ffacb1553d2
Since We can use EditorBase/TextEditor/HTMLEditor directly, we can
movei noscript methods in nsIEditor to each class.
Also, Init is unnecessary to use nsIDOMDocument and nsIContent since method
isn't in IDL. And some methods are unused now.
MozReview-Commit-ID: D3B6oSlcT0L
--HG--
extra : rebase_source : 6cab2e6e7b4ba8cfb56d8320be24ca4afcbe55fb
extra : amend_source : 1d8c59086a9158a49dd270b64ecf8341ed4002ce
Before bug 1425091, we called RangeUpdater::SelAdjCreateNode() with point of
new node in EditorBase::CreateElement(). However, it was accidentally changed
to point *in* new element. Therefore, setting such DOM point causes warnings
in debug build and call of RangeUpdater::SelAdjCreateNode() must be failed
since the point stores wrong container node.
Additionally, this patch stops using another EditorRawDOMPoint instance which
is just redundant and renames |ret| to |newElement| for making its meaning
clearer.
MozReview-Commit-ID: Gc0YgaNLYcx
--HG--
extra : rebase_source : c2fbe2669ca8a5993e26b357f53dc38341fd6bf6
Since We can use EditorBase/TextEditor/HTMLEditor directly, we can
movei noscript methods in nsIEditor to each class.
Also, Init is unnecessary to use nsIDOMDocument and nsIContent since method
isn't in IDL. And some methods are unused now.
MozReview-Commit-ID: D3B6oSlcT0L
--HG--
extra : rebase_source : b89698f4ab56fdd97a4cd8c515a8b33f6a74a7af
I left some IgnoredErrorResults for now where people warn on failure. We could
consider adding a WarnOnError() thing or something.
MozReview-Commit-ID: L5ttZ9CGKg0
This patch makes EditorBase derived from nsISelectionListener. Then, we can
make IMEContentObserver, TextInputListener, ComposerCommandsUpdater,
TypeInState not derived from nsISelectionListener since EditorBase or
HTMLEditor can notify them of selection change directly. Additionally,
ResizerSelectionListener is not necessary anymore since it just implements
nsISelectionListener and calls only a method of HTMLEditor. So, HTMLEditor
can call it directly.
Note that the order of selection listeners may be different. However,
according to what each selection listener does, changing the order isn't
problem.
MozReview-Commit-ID: 1JXZxQcS0tP
--HG--
extra : rebase_source : c2ebe622a74001ad4e421da492dcdab8e6fe1649
Now, nobody implements nsIEditorObserver::BeforeEditAction() and
nsIEditorObserver::CancelEditAction(). So, we can remove them.
MozReview-Commit-ID: DYppLayC5oT
--HG--
extra : rebase_source : 196e042234c65a43947a1aa90898901389a3a53b
Similar to TextInputListener, EditorBase should store IMEContentObserver
directly instead of via nsIEditorObserver. Then,
EditorBase::NotifyEditorObservers() can call each method directly.
Additionally, we can make IMEContentObserver not derived from nsIEditorObserver.
MozReview-Commit-ID: cNKWJe5eUC
--HG--
extra : rebase_source : 4ed3b3b3180b8ee4a7c514ce1f89eba4dad64fbe
Now, EditorBase can store TextInputListener directly instead of as
nsIEditorObserver. And then, EditorBase can call its EditAction() method
directly. Therefore, we can make TextInputListener not derived from
nsIEditorObserver.
MozReview-Commit-ID: 4qPnnvReLKy
--HG--
extra : rebase_source : cde47e245c9856abf696dbaf8e26d8e4d6d98d42
mozInlineSpellChecker listens to only DidSplitNode() and DidJoinNodes().
So, EditorBase should call them directly rather than treating it as an
nsIEditActionListener since its runtime cost becomes really cheaper.
Different from EditorSpellCheck, nobody creates instance of
nsIInlineSpellChecker. So, we can now stop supporting createInstance() for it
in chrome JS and should do this since EditorBase cannot support multiple
mozInlineSpellChecker instances without nsIEditActionListener interface.
Therefore, this patch removes the entry from factory.
MozReview-Commit-ID: W1CLdsJaaB
--HG--
extra : rebase_source : 2319999e4b1fc8978386c49236d5d24d78d86047
Currently, first edit action listener is always TextServicesDocument for
EditorBase::mInlineSpellChecker if spell check is enabled and it requires
only DidDeleteNode() and DidJoinNodes(). So, in most cases, EditorBase
should call only them to avoid running redundant for loops for
nsIEditActionListener methods.
This patch makes that EditorBase::AddEditActionListener() and
RemoveEditActionListener() check whether coming nsIEditActionListener is
TextServicesDocument for mInlineSpellChecker. If so, EditorBase should
store it as reference to TextServicesDocument. And when edit action occurs,
its DidDeleteNode() and DidJoinNodes() should be called directly.
Unfortunately, this patch makes TextServiceDocument's maintaining rules
complicated. However, it must be really rare case to add new edit action
listener because it's really old component.
Note that EditorSpellCheck may be created multiple instances for an editor
from chrome JS. Therefore, we need to keep TextServicesDocument being
derived from nsIEditActionListener because in such case, TextServicesDocument
for other EditorSpellCheck instances should be supported via
nsIEditActionListener even though such case makes EditorBase slower.
MozReview-Commit-ID: KtlXsBCOzKL
--HG--
extra : rebase_source : 6827ed09a028f2049fd7afba2f5116d092bd14e5
EditorBase::mInlineSpellChecker is always an instance of mozInlineSpellChecker.
Fortunately, it's easy to expose mozInlineSpellChecker.h. So, making EditorBase
store it as mozInlineSpellChecker directly, EditorBase can access any of
mozInlineSpellChecker, EditorSpellCheck, mozSpellChecker and
TextServicesDocument with new accessors created by following patch.
MozReview-Commit-ID: 2oyS5tPeQcg
--HG--
extra : rebase_source : a9ce2e4dbceff7ca800d34d60d56eba184298677
Most nsIEditActionListener::Will*() are not implemented, except
WillDeleteText() and WillDeleteSelection() which are implemented by
FinderHighlighter. So, we can get rid of the other Will*() from it.
This patch removes a lot of unnecessary virtual calls and copy of strong
pointers to edit action listeners of EditorBase.
MozReview-Commit-ID: EsqI2tZoBG1
--HG--
extra : rebase_source : cf78eb8d33b12ca65177b0676f6e45d02e7c0688
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
AutoMoveNodeSelNotify doesn't allow container is null. So we should reject
that parent of moving node is null.
MozReview-Commit-ID: DGNCicLJxuK
--HG--
extra : rebase_source : eea203f16d0296c74d0727f34b6ddc5c165f9092
This crashtest case hits assertion in SelAdjInsertNode due to invalid offset
point. So we check whether this is valid.
Also, SelAdjCreateNode and SelAdjInsertNode should use EditorRawDOMPoint to avoid offset calculation and check valid offset.
MozReview-Commit-ID: A1kaKEzc36e
--HG--
extra : rebase_source : c46843fdd6e9e18f49b9cedb008cda6cf3436811
Although, we don't need factory methods for DeleteRangeTransaction,
EditAggregateTransaction nor PlaceholderTransaction, for consistency with the
other transaction classes, they should have factory methods for making easier
to write the code.
For not making the performance slow down, they should be inline methods.
MozReview-Commit-ID: 7jl5yZNFYmP
--HG--
extra : rebase_source : 7cd5b5e268a670b3c8855407cc72dec12d34d8ff
This patch creates factory methods for AddStyleSheetTransaction and
RemoveStyleSheetTransaction, and removes EditorBase::CreateTxnForAddStyleSheet()
and EditorBase::CreateTxnForRemoveStyleSheet() instead.
MozReview-Commit-ID: 6dnZctDtNik
--HG--
extra : rebase_source : 43eaadbde06e4a0b061ea8136e12ffeccfaf5592
This patch creates two factory methods of ChangeAttributeTransaction. One is
for setting an attribute to specific value. The other is for removing an
attribute. So, EditorBase::CreateTxnForSetAttribute() and
EditorBase::CreateTxnForRemoveAttribute() are unnecessary anymore.
MozReview-Commit-ID: 2fEVd3pDXsf
--HG--
extra : rebase_source : 674005a5b9fc623999a0f51dc8697027970f06c9
EditorBase::CreateTxnForJoinNode() just hides what it does.
For making the caller clearer, let's create a factory method,
JoinNodeTransaction::MaybeCreate().
MozReview-Commit-ID: 8vADXdzMeuV
--HG--
extra : rebase_source : 6a281aff11bfa019c292d26cadd0cd29da12753f
