EditorBase::GetSelection() sometimes appears in profile. So, it shouldn't be
called in nsIEditor::EndOfDocument() if the callers of
nsIEditor::EndOfDocument() has a pointer to Selection.
This patch adds EditorBase::CollapseSelectionToEnd() for the internal method
and make all callers of nsIEditor::EndOfDocument() use it.
MozReview-Commit-ID: 8H4ThxzdKDf
--HG--
extra : rebase_source : 7436d7f286351fa9c91175b40e686f7ca1f3c69f
In most cases, TextEditRules::CollapseSelectionToTrailingBRIfNeeded() doesn't
use the index anymore. Therefore, it should stop using
EditorBase::GetNodeLocation() and use EditorBase::GetChildOffset() only when
the offset is necessary.
MozReview-Commit-ID: 9vGcLnTUnsu
--HG--
extra : rebase_source : af0cdad3a1905bf2e5654a109aa4037ec70b422b
nsINode::GetChildAt() is too expensive if it's used for next sibling of a node.
This patch makes it use nsINode::GetNextSibling() instead.
MozReview-Commit-ID: CAC7gFNo77w
--HG--
extra : rebase_source : 185457b910e2e09c58a6f02525ae61aff149511c
nsINode::IndexOf() is expensive especially when it's in a hot path, it's too
expensive. So, EditorBase::GetChildOffset() and EditorBase::GetNodeLocation()
should check child's siblings first. If some of them are nullptr, it means
that it's first child or last child of the parent.
Note that EditorBase::GetChildOffset() may return wrong index if it's called
while aChild is being removed from aParent or aParent isn't actual parent of
aChild. However, there are MOZ_ASSERTs to ensure value isn't -1. Therefore,
it's safe to assume that aParent is always the parent of aChild and it won't
be called from ContentRemoved() of mutation observers.
MozReview-Commit-ID: 8JdYWuZbHe5
--HG--
extra : rebase_source : 6ded91a3aa8b00ab4d2d544c2c392d88cb769cef
These are all easy cases where an nsXPIDLCString local variable is set via
getter_Copies() and then is only used in ways that nsCStrings can also be used
(i.e. no null checks or implicit conversions to |char*|).
In every case the patch trivially replaces the nsXPIDLCString with an
nsCString. (Also, there are a couple of unused nsXPIDLCString variables that
the patch simply removes.)
test_bug655636.html refers gBrowser, however, it's available only in chrome
process and it's referred only for listening to "pageshow" event instead of
"load" event of the data URI. So, we must be able to use "unload" event of the
previous URL instead.
Although, this testcase (even without this change) won't cause crash even if
backing out the patch for bug 635636 anymore.
MozReview-Commit-ID: B8qOwVZqZQm
--HG--
extra : rebase_source : d383181886152684a8bf9c2caf7248d5f7582c0a
AutoPlaceHolderBatch can take EditorBase class and its inherited class, AutoEditBatch, can be removed if we implement other constructor which doesn't take transaction name.
Additionally, nsIEditor::(Begin|End)PlaceHolderTransaction() are referred only by AutoPlaceHolderBatch. Therefore, they can be non-public methods and removed from nsIEditor interface.
Note that this patch also repalces "PlaceHolder" with "Placeholder" since it's a word.
MozReview-Commit-ID: 5dw3kcX3bOx
--HG--
extra : rebase_source : e926cc1c2ebea70eb08e43778a8b52912b559b7b
nsITableEditor is now a builtin class. So, it's implemented only by HTMLEditor. Therefore, AutoSelectionSetterAfterTableEdit can use HTMLEditor.
Then, nsITableEditor.setSelectionAfterTableEdit() can be removed from nsITableEditor and moved to HTMLEditor as non-virtual method since nobody uses it from JS.
MozReview-Commit-ID: KnN6Fw4TYyn
--HG--
extra : rebase_source : 48412a5f81f30d9ada47550fdb4d1ee0d88de6f4
This test is turned off by bug 1269209 on e10s. But there is no exactly reason why this test on e10s is turned off.
Although I run this test on e10s now, it seems to pass this. So we should turn on this test even if e10s.
MozReview-Commit-ID: JHQP2ZYvsHL
--HG--
extra : rebase_source : 2bc45f0d2b5cc99bad93267f40191866264af414
nsIHTMLDocument::TearingDownEditor() takes nsIEditor as an argument. However, it's not used in it. So, we can remove it.
MozReview-Commit-ID: KUmIsnVGB6H
--HG--
extra : rebase_source : ddd344b9df18e0b33d54d2f99e6335449f55df81
nsEditingSession should treat editor as HTMLEditor in its internal handling.
MozReview-Commit-ID: 51SfY4oeq3p
--HG--
extra : rebase_source : 90535267e62de17755fff9f21b6a0f74244d849a
Some users of HTMLEditor retrieve pointer to editors with nsIEditingSession::GetEditorForWindow() but it returns nsIEditor. So, there should be a method which returns HTMLEditor*.
MozReview-Commit-ID: Bzc1LIlTSDF
--HG--
extra : rebase_source : f6fb3e5852809487883e3e6fedda9580206b2cb5
This matches the spec and Chrome, and seems to bring us closer to Edge
and WebKit as well. It also matches our own behavior for addRange(),
which was changed in bug 1341137.
For collapse and selectAllChildren, we match the tests and browsers, but
the spec is incorrect at the time of this writing:
https://github.com/w3c/selection-api/pull/86
The removeAllRanges test hadn't been updated for the spec change.
MozReview-Commit-ID: DTK8283k5IP
--HG--
extra : rebase_source : 54701e7136c33ebce651d5f74c3dc1d8b944f9a3
Removes nsIDOMHTMLEmbedElement and all references. HTML elements are
now handled by WebIDL. With the deprecation of extensions, XPCOM
interfaces to HTML elements are no longer needed.
MozReview-Commit-ID: DI4XVvdgPDI
--HG--
extra : rebase_source : 74bd92619e3d1db04c3dd40ec3022474fe1d647c
EditorCommands implements common edit commands. So, we can use TextEditor even if the instance is HTMLEditor.
Then, EditorCommands can use non-virtual methods of the concrete classes.
MozReview-Commit-ID: DgKHqC0osRb
--HG--
extra : rebase_source : e9abd7ef8b48cfdc65bbeb56ebe81dc0548005dd
nsComposerCommands uses some simple getter methods. They can be simpler non-virtual methods. So, we should do it.
Note that this changes that EditorBase::GetIsSelectionEditable() won't return error. However, it has returned error only when selection controller isn't available. That means that the selection controller has been destroyed and the editor will be destroyed. So, this must not be problem since it returns false (non-editable) instead and won't break any behavior since the editor won't be editable by users nor JS anymore.
MozReview-Commit-ID: E9ccFspG6na
--HG--
extra : rebase_source : bcd1314cb386fcaf175adabfefde5885decd87c0
Compiler may can optimize to call virtual methods at build time if we call them with concrete classes because some of them may have final keyword.
Even if not so, we can optimize some methods with creating non-virtual methods.
MozReview-Commit-ID: K3bRlc0URml
--HG--
extra : rebase_source : 4a76635c7aed29501f71ae74f3f73e2b22ca219e
All SetState() methods in nsComposerCommands require HTMLEditor. So, it should take HTMLEditor* rather than nsIEditor*.
MozReview-Commit-ID: AVbnRsMsmeY
--HG--
extra : rebase_source : 23bcc585a045a39620102c4fef9c1a9e12b9ea37
Similar to GetCurrentState(), all ToggleState() methods require HTMLEditor. So, they should take HTMLEditor* instead of nsIEditor*.
MozReview-Commit-ID: BwM6WRKFn6Q
--HG--
extra : rebase_source : 425b7b405b5881b97e8113c3ab88e9d0514b6a07
All GetCurrentState() methods in nsComposerCommands require HTMLEditor but its argument is nsIEditor*. So, it should take HTMLEditor* and it shouldn't be called if given editor isn't HTMLEditor since it's virtual method.
MozReview-Commit-ID: HsvYJN8hIxN
--HG--
extra : rebase_source : f8f9c8de902af5311771b71a96c76d63325970a5
nsIEditor is still first contact with editor class for some modules. They should be accessible to the concrete classes without QI. Therefore, nsIEditor should have As*Editor() methods.
Additionally, this adds AsEditorBase(). That is always implemented but it might be necessary for some files for minimizing its include files.
MozReview-Commit-ID: 8WqkDJLiVDs
--HG--
extra : rebase_source : e51282df244efad62bc6fe04ab449e3beab440f9
CompositionTransaction forgets to call EndBatchChanges when GetSelectionController returns error, so we should use RAII class instead.
MozReview-Commit-ID: HI9kutRVzx6
--HG--
extra : rebase_source : d276f4349c5a64d4610581ae1a76d160841f7d7b
If we make nsIEditor a builtin class, that means that its instance can be only TextEditor or HTMLEditor. Then, users of nsIEditor can use concrete classes such as EditorBase, TextEditor or HTMLEditor instead. Then, the users can reduce unnecessary QI and a lot of virtual calls if we'll create non-virtual methods.
So, let's make editor related interfaces builtin classes.
MozReview-Commit-ID: 93WfsSDuJiJ
--HG--
extra : rebase_source : 39151eb4ffaaf195fec57234ea84c9f4066c58d1
nsXPIDLStrings are marked as VOIDED upon initialization. Most of these local
nsXPIDLString variables are immediately set via getter_Copies(), which will
either assign a string value (using Adopt()) or do SetIsVoid(). These can be
trivially converted to nsString, which will get the same treatment.
The patch suitably converts the remaining nsXPIDLString local variable as well.
--HG--
extra : rebase_source : 5fff9f2c6844559198f601853f8db08564add7d5
CreateAnonymousElement still uses nsIDOMNode, but we should use nsIContent for it.
MozReview-Commit-ID: 2xgzlE6NVra
--HG--
extra : rebase_source : c93a9acb230604da391f705b0038e3cf7fbd4f23
This method can be extremely hot, so we need to remove all sources of XPCOM
overhead from it. This includes the usages of weak pointers (thanks to the
previous parts), refcounting, and QueryInterface.
I kept the callers hold the selection controller alive by assigning the
return value to an nsCOMPtr in places where the methods called on it could
have a remote chance of messing with the lifetime of objects.
Besides some unnecessary copying and malloc overhead that this removes, it
also removes a call to dom::Text::GetData() which we used to only make to
get the old length of the text node before modifying it. Turns out that
this old length was already obtained once in SetTextImpl().
Currently, HTMLEditRules::SplitParagraph() inserts moz-<br> element when split element and/or new element causes empty line. However, PlaintextSerializer ignores moz-<br> elements. Therefore, empty lines which are created by SplitParagraph() will be removed when it's converted to plaintext.
So, it should insert normal <br> element for placeholder of empty lines instead. Note that moz-<br> elements are appeared as normal <br> elements in the result of Element.innerHTML. Additionally, Chromium always inserts a <br> element for empty block elements which are created by Enter key press. Therefore, using normal <br> element in this case shouldn't cause any compatibility problems.
MozReview-Commit-ID: FNV41zEFWqQ
--HG--
extra : rebase_source : ece6c2e275e2a75d9d2871d62ed4204115792b99
After reframe, SetSelectionAfterTableEdit might be called from RAII class, so we should check whether editor is destroyed.
MozReview-Commit-ID: I4r6kvxgRDZ
--HG--
extra : rebase_source : 4bd71775f02f57573af0f0efd5c1946285e4e4f4
HTMLEditor::TabInTable inserts row element, then it selects a cell. But when enabling lazy frame construction for editable node, it selects invalid cell and table.
Because HTMLEditor::SetSelectionAfterTableEdit doesn't select cell correctly on InsertTableRow().
HTMLEditor::SetSelectionAfterTableEdit uses HTMLEditor::GetCellAt, so it depends on frame. So we need flush frame before calling it.
Also, a comment of HTMLEditor::InsertTableRow is invalid now because we don't use nsresult version of CreateElementWithDefualts.
MozReview-Commit-ID: 698TvmMZgwB
--HG--
extra : rebase_source : 95df12f1f870a2c68d02b24b8758dfe6d381a416
This makes the SetTextTransaction transaction behave more similarly to the rest of
the editor transactions, by making sure the inner transactions don't manipulate
the selections themselves and leave it up to the AfterEdit() method to take care of
adjusting the selection when the entire editing operation is finished.
test_bug795418-*.html depends on clipboard. But these tests aren't subsuite=clipboard and are sometimes failure (bug 1334700, bug 1335221, bug 1339430, bug 1343883 and bug 1348093). So we should move these to subsuite=clipboard to try fixing this.
MozReview-Commit-ID: 1m5dFGZ18a
--HG--
extra : rebase_source : 847b1b7842c32e93c61b8ee572cff16c70adc323
nsIDocument::AdoptNode might to remove all properties. So before destroying editor, NAC property might be already removed. So we have to consider that NAC is removed.
MozReview-Commit-ID: DakRRqrzEtF
--HG--
extra : rebase_source : ce972f1b42b6ff7efdf09b9a70cc1a9d27f128fb
I've been having problems with interdiffs on mozreview lately, so for
ease of review, this patch is being submitted as a seperate patch for
review. Once it is r+'d, it will be folded into the first patch in
this set before landing.
MozReview-Commit-ID: CS9MngaXlBd
--HG--
extra : rebase_source : 6a86fd4f7a66e73497a756976a2562d183002a2a
This is similar like the previous patch, but for the 8-bit string variants.
Also, it changes assignment to Adopt() in GetCString() and GetDefaultCString()
to avoid an extra copy.
--HG--
extra : rebase_source : eba805c3a7b809d5ccd6e853b1c9010db9477667
Because we want to remove nsAdoptingString. We have other variants that don't
use nsAdoptingString, which can be used instead. There are three basic
patterns.
1. The easiest case is when we don't check for success.
> nsAdoptingString s = Preferences::GetString("foo");
> foo(s);
becomes:
> nsAutoString s;
> Preferences::GetString("foo", s);
> foo(s);
2. The next case is when we check if the result is empty.
> nsAdoptingString s = Preferences::GetString("foo");
> if (s.IsEmpty()) { ... }
becomes:
> nsAutoString s;
> Preferences::GetString("foo", s);
> if (s.IsEmpty()) { ... }
3. The final case is when we null check the result.
> nsAdoptingString s = Preferences::GetString("foo");
> if (s) { ... }
becomes:
> nsAutoString s;
> nsresult rv = Preferences::GetString("foo", s);
> if (NS_SUCCEEDED(rv)) { ... }
The patch also avoids some UTF8/UTF16 conversions in a few places.
--HG--
extra : rebase_source : f339b1a3dda4dc93979d38c30c001fbe77485b55
This is basically a cosmetic change; references are the normal way to do string
outparams.
--HG--
extra : rebase_source : ffc5945f269bdcd3d4116755b56713e87a44b6cd
This patch replaces four functions of the name AssignWithConversion which
are essentially wrappers around CopyASCIItoUTF16 and LossyCopyUTF16toASCII
with direct calls to the latter two functions. The replaced functions are:
void nsCString::AssignWithConversion( const nsAString& aData )
void nsString::AssignWithConversion( const nsACString& aData )
void nsTString_CharT::AssignWithConversion(
const incompatible_char_type* aData,
int32_t aLength = -1);
The last of the three exists inside the double-included nsTString* world and
so describes two functions, giving four in total.
This has two advantages:
* it removes code
* at the call points, it makes clear (from the replacement name) which
conversion is being carried out. The generic name "AssignWithConversion"
doesn't make that obvious -- one had to infer it from the types.
The patch also removes two commented out lines from
editor/composer/nsComposerCommands.cpp, that appear to be related. They are
at top level, where they would never have compiled. They look like
leftovers from some previous change.
--HG--
extra : rebase_source : fb47bf450771c3c9ee3341dd14520f5da69ec4f5
It is mainly for avoiding other anonymous element being inserted into
<svg:use>, but in general we probably don't want to insert some random
HTML anonymous element into a non-HTML element.
MozReview-Commit-ID: J5kCUzA7K90
--HG--
extra : rebase_source : 1d38cd9297071b9ba8151eb27d74b56ad540cc49
DOM Standard defines that offset of Range is unsigned long. However, nsRange uses int32_t to them.
This patch makes nsRange use uint32_t instead. However, this patch does NOT allow to set over INT32_MAX as offset values since a lot of users of nsRange cannot treat the values as over INT32_MAX because a lot of internal APIs take int32_t as offsets.
For easier to search such points, this patch adds static_cast<int32_t> to uint32_t variables when they are used for int32_t arguments.
And note that nsContentUtils::ComparePoints() behaves odd. It accepts negative offset and compares such value with valid offset simply. This patch still uses int32_t offset variables in nsRange::CompareNodeToRange() even though it may be negative value if nsINode::IndexOf() returns -1 because the caller of it depends on this behavior.
MozReview-Commit-ID: 8RbOgA86JuT
--HG--
extra : rebase_source : 46d526c6d50dfa2f104439b19b8691477b17a4af
When we turn on the pref security.data_uri.unique_opaque_origin to make
data URI is unique origin, the following tests will fail because now
data document is treated as cross origin, so we replace data URIs with
seperate files, hence it will still in the same origin.
Add minimal crash test for this. When removing a line, this doesn't occur.
MozReview-Commit-ID: JqS2E8q47ML
--HG--
extra : rebase_source : 322a77cbc49e29bbd6b705650485634dd4801b8b
When using EditAction::insertBreak action, if selection node isn't content, this crash occurs. So we should reject to promote range when it isn't content node.
MozReview-Commit-ID: AJ4oEOmRTgm
--HG--
extra : rebase_source : 04c4e4a2e04a0d0a058eb86fdfde0005edb681a9
GetPromotedPoint still uses nsIDOMNode, so we should use nsINode etc instead for clean up.
MozReview-Commit-ID: 4efe46iU7lC
--HG--
extra : rebase_source : bb13c4531fb76684d4baf13e24dc777d79c2ad61
This does NOT change variable names like |endNode| because it's not odd and somebody use it for nsINode and endContent for nsIContent. So, changing them needs more work.
MozReview-Commit-ID: 22imUltlu5R
--HG--
extra : rebase_source : 6c93069d0586b37c5084eaa71861085c01da7a7d
This does NOT change variable names like |startNode| because it's not odd and somebody use it for nsINode and startContent for nsIContent. So, changing them needs more work.
MozReview-Commit-ID: H19pTDprRuT
--HG--
extra : rebase_source : 7a7454ac14da48a597ff19a50c863d04dcaddd6e
Web standards use "Container" instead of "Parent". So, nsRange shouldn't use "Parent" for its members and methods.
MozReview-Commit-ID: Ho6N0diuWtE
--HG--
extra : rebase_source : ee4eb7068a68b118c7fe98e9e9e7fa9c9e7f13e2
When setting empty value by input.value, we remove text node and insert bogus node. But creating and removing node are expensive. So we should keep text node for performance if possible.
Now, DocumentIsEmpty only checks bogus node to detect empty. So, keeping text node change causes that document cannot detect as empty. If root has only text node and all is empty, we should detect empty document.
This change should be only plain text editor. HTML editor already allows multiple text nodes, so we should keep old behaviour on HTML editor.
MozReview-Commit-ID: Gt8GmdWAA3E
--HG--
extra : rebase_source : 4c5deba024cab3d7e2e1e2a8ec53a29f2fdf8cd9
