HTMLEditRules::WillDeleteSelection is complex since some variables is reused.
So I would like to clean up this to use block scope and EditorDOMPoint
before fixing bug 685799.
Differential Revision: https://phabricator.services.mozilla.com/D3761
--HG--
extra : moz-landing-system : lando
RemoveList doesn't use aListType parameter, but c-c still call this method.
So I keep aListType parameter even if unused.
Differential Revision: https://phabricator.services.mozilla.com/D3762
--HG--
extra : moz-landing-system : lando
HTMLEditor::RefereshEditingUI() works only with enabled UIs. Therefore, if
UI is disabled while it's visible, it keeps shown. This is too bad if web
apps tries to disable the Gecko specific UIs after we show some of them.
This patch adds HTMLEditor::HideAnonymousEditingUIsIfUnnecessary() to hide
unnecessary UIs and makes RefereshEditingUI() call it always.
First, HTMLEditor::HideInlineTableEditingUI() always returns NS_OK. So, we
can change its return type to void.
Additionally, it removes each UI from the DOM tree one by one. However, each
mutation could cause showing same UI again. In such case,
ShowInlineTableEditingUI() overwrites each UI with newly created element.
Then, HTMLEditor cannot remove the old UI anymore. Therefore, this patch
moves all members of the UI into local variables first.
HTMLEditor::CheckSelectionStateForAnonymousButtons() is called a lot internally.
Especially, its virtual call cost may make damage to our performance since
it's called from a selection listener.
So, we should create non-virtual method, RefereshEditingUI() for internal use.
Even after we disable Gecko specific editing UIs by default, web apps can
enable them with execCommand. However, until such web apps change their
behavior, users cannot use Gecko specific UIs. At least for now, we should
make users can enable them by default.
MozReview-Commit-ID: AuAdw4FQ4He
--HG--
extra : rebase_source : a1f88f2928df0d7afb4361c425d75c74872ac9d5
Currently, absolute position editor listens to mouse events at the default
event group to handle dragging of positioner. However, this is blocked by
a call of Event.stopPropagation() in web apps unexpectedly. Therefore,
we should make it listen to the events at the system event group instead.
MozReview-Commit-ID: Hoa8c9QvMuG
--HG--
extra : rebase_source : 77500356fd1a65e8d81da131e09bc48229a208f9
We have another built-in UI of editor which is not implemented by any other
browsers. That is a draggable handler to move absolute positioned elements.
So, we should disable it in default for compatibility with the other browsers.
However, different from resizers and inline table editor, we don't have
command to enable/disable this feature but for backward compatibility, we
should have it. Therefore, this patch adds new command
"enableAbsolutePositionEditor".
Note that whether resizing UI is available only with enableObjectResizing
state is different from enableInlineTableEditing command. Resizers for
absolute positioned elements are NOT available both enableObjectResizing
and enableAbsolutePositionEditor are enabled.
Additionally, this adds automated tests to check basic functions of absolute
positioned editor.
MozReview-Commit-ID: 9ZSGB8tLpFw
--HG--
rename : editor/libeditor/tests/test_resizers_appearance.html => editor/libeditor/tests/test_abs_positioner_appearance.html
rename : editor/libeditor/tests/test_resizers_resizing_elements.html => editor/libeditor/tests/test_abs_positioner_positioning_elements.html
extra : rebase_source : d516f3f3ef36d4ad13938f214cb6e3868d7ff407
Gecko supports resizers of <img> elements and <table>, <td>, <th> elements and
has UI to remove existing table row or column in default. However, the other
browsers don't have such UI and web apps need to disable this feature with
calling both:
document.execCommand("enableObjectResizing", false, false);
document.execCommand("enableInlineTableEditing", false, false);
for avoiding conflicting with their own features to edit such elements.
Therefore, it doesn't make sense to keep enabling them in default only on
Gecko. If web apps want to keep using these features, they should call:
document.execCommand("enableObjectResizing", false, true);
document.execCommand("enableInlineTableEditing", false, true);
at initializing the editor.
And also this patch fixes bugs of
document.queryCommandState("enableObjectResizing") and
document.queryCommandState("enableInlineTableEditing"). They always return
false even after calling document.execCommand(..., false, true) since
nsSetDocumentStateCommand::GetCommandStateParams() sets bool value as
STATE_ATTRIBUTE. However, nsHTMLDocument::QueryCommandValue() which is the
caller referring STATE_ATTRIBUTE doesn't treat it as bool value. And also
those commands are related to state of document. Therefore, they should be
return as bool value of STATE_ALL instead. Then,
nsHTMLDocument::QueryCommandState() returns the state as expected. Note that
those commands are supported only by Gecko. So, we don't need to worry about
the compatibility.
Finally, this patch rewrites 2 existing tests to check basic behavior of
resizers and appearance of resizers.
Note that this patch does not add new tests to test inline table editor
since it's difficult to test the behavior with current API. Perhaps, we
should add an API to nsIHTMLEditor to retrieve each anonymous elements in
another bug since it requires to add wrapping API of SpecialPowers.
MozReview-Commit-ID: 1FhYo5vcV60
--HG--
rename : editor/libeditor/tests/test_objectResizing.html => editor/libeditor/tests/test_resizers_appearance.html
rename : editor/libeditor/tests/test_bug640321.html => editor/libeditor/tests/test_resizers_resizing_elements.html
extra : rebase_source : a707de5a64ef1f8ce974cdf1be093d1b4f61c7bc
The methods compared with const characters since we've supported "namedanchor"
which is not in nsGkAtoms. Now, it's dropped so that we can compare given
atom with nsGkAtoms.
Differential Revision: https://phabricator.services.mozilla.com/D3586
--HG--
extra : moz-landing-system : lando
Nobody (including comm-central and BlueGriffon) does not use "namedanchor"
special element name with those XPCOMs. Of course, our internal callers too.
Therefore, we can drop.
Note that there is no static Atom for this, so, keeping it makes unnecessary
runtime cost for Firefox users.
This could cause breaking some legacy add-ons for Thunderbird. However,
they can use "anchor" special element name for same purpose.
Differential Revision: https://phabricator.services.mozilla.com/D3585
--HG--
extra : moz-landing-system : lando
HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&)
and IsNamedAnchorTag(const nsAString&). For making their maintenance easier,
let's make GetElementOrParentByTagName() take const nsAtom& for tag name.
GetElementOrParentByTagName() has two functions, one is looking for an element
starting from a node. The other is, if the start node is nullptr, it retrieves
anchor node of Selection as start node. Therefore, this patch splits the
first part to GetElementOrParentByTagNameInternal(). Then, creates its
wrapper which retrieves anchor of Selection automatically,
GetElementOrParentByTagNameAtSelection().
Additionally, this patch makes all internal callers of HTMLEditor use
GetElementOrParentByTagNameInternal() or
GetElementOrParentByTagNameAtSelection() directly. Then, public method,
GetElementOrParentByTagName() is called only by outer classes.
Note that some callers use both GetElementOrParentByTagNameInternal()
and GetElementOrParentByTagNameAtSelection() since they don't check whether
setting node is nullptr. They may be bug of them. We should investigate
the API callers later.
Differential Revision: https://phabricator.services.mozilla.com/D3584
--HG--
extra : moz-landing-system : lando
Nobody (including comm-central and BlueGriffon) does not use "namedanchor"
special element name with those XPCOMs. Of course, our internal callers too.
Therefore, we can drop.
Note that there is no static Atom for this, so, keeping it makes unnecessary
runtime cost for Firefox users.
This could cause breaking some legacy add-ons for Thunderbird. However,
they can use "anchor" special element name for same purpose.
Differential Revision: https://phabricator.services.mozilla.com/D3585
--HG--
extra : moz-landing-system : lando
HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&)
and IsNamedAnchorTag(const nsAString&). For making their maintenance easier,
let's make GetElementOrParentByTagName() take const nsAtom& for tag name.
GetElementOrParentByTagName() has two functions, one is looking for an element
starting from a node. The other is, if the start node is nullptr, it retrieves
anchor node of Selection as start node. Therefore, this patch splits the
first part to GetElementOrParentByTagNameInternal(). Then, creates its
wrapper which retrieves anchor of Selection automatically,
GetElementOrParentByTagNameAtSelection().
Additionally, this patch makes all internal callers of HTMLEditor use
GetElementOrParentByTagNameInternal() or
GetElementOrParentByTagNameAtSelection() directly. Then, public method,
GetElementOrParentByTagName() is called only by outer classes.
Note that some callers use both GetElementOrParentByTagNameInternal()
and GetElementOrParentByTagNameAtSelection() since they don't check whether
setting node is nullptr. They may be bug of them. We should investigate
the API callers later.
Differential Revision: https://phabricator.services.mozilla.com/D3584
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetSelectionContainer() is a public method, but it's not used by
outer classes. So, we can make it a protected member.
Additionally, this patch cleans up the method.
- Renames to GetSelectionContainerElement() for making clearer what will be
returned.
- Makes it const.
- Makes it take Selection reference since most callers already have Selection.
- Makes it use RangeBoundary to access start point and end point of range since nsRange::StartOffset() and nsRange::EndOffset() may be slow.
- Makes it not use GetSelectedElement() since it requires unnecessary additional cost and the condition to call it means it uses only the first path in GetSelectedElement() which just returns start node of the range.
- Makes it output warning when it returns nullptr since it reaches nullptr only when illegal cases, e.g., Selection is in orphan node.
Differential Revision: https://phabricator.services.mozilla.com/D3461
--HG--
extra : moz-landing-system : lando
EditorBase::AreNodesSameType() is overridden only by HTMLEditor and the
implementation is enough simple to re-implement in EditorBase.
Additionally, this is called from condition of a loop in
JoinNodesDeepWithTransaction(). So, the virtual call cost may make damage
to the performance.
Differential Revision: https://phabricator.services.mozilla.com/D3460
--HG--
extra : moz-landing-system : lando
HTMLEditor::SetIsCSSEnabled() is an XPCOM but it's defined with nsresult.
Differential Revision: https://phabricator.services.mozilla.com/D3459
--HG--
extra : moz-landing-system : lando
HTMLEditor::EnableStyleSheet() is an XPCOM method but it's used internally.
Therefore, we should create non-virtual method for internal use.
Differential Revision: https://phabricator.services.mozilla.com/D3456
--HG--
extra : moz-landing-system : lando
HTMLEditor::RemoveOverrideStyleSheet() is an XPCOM method but used internally.
So, we should create non-virtual method for this.
Additionally, it calls GetStyleSheetForURL() and RemoveStyleSheetFromList(),
but they search index of internal override style sheet array redundantly.
Moreover, RemoveStyleSheetFromList() returns error only when given URL is
not found, but RemoveOverrideStyleSheet() which is the only one caller, ignores
the error. Therefore, for saving the redundant cost, this patch makes
RemoveStyleSheetFromList() return removing StyleSheet which is retrieved
with the call of GetStyleSheetForURL(). So, RemoveOverrideStyleSheetInternal()
stops calling GetStyleSheetForURL().
Differential Revision: https://phabricator.services.mozilla.com/D3455
--HG--
extra : moz-landing-system : lando
HTMLEditor::AddOverrideStyleSheet() is an XPCOM method but it's called
internally. So, we should create non-virtual method for it and call it
for internal use.
Differential Revision: https://phabricator.services.mozilla.com/D3454
--HG--
extra : moz-landing-system : lando
Fortunately, despite of becoming public method,
HTMLEditor::CreateElementWithDefaults() can be used by internal methods too
since it does not touch undo transactions nor the DOM tree, and does not
refer mRules nor GetSelection(). So, we can make it public and make any
C++ callers use it.
If HTMLEditor::GetSelectedNode() is called with nullptr for aTagName,
the first block may return non-element node. In such case, we should return
nullptr without error for now (since I have no idea which element node is
a good node to return).
Then, we can rename it to GetSelectedElement() and can replace existing
GetSelectedElement() with the new one.
The first check of the last block of HTMLEditor::GetSelectedNode() can use
early-return style. Additionally, |current| is not necessary since Selection
is not changed since the method retrieved first range. So, we can get rid of
|current| and the |nullptr| case of it.
Some variables in HTMLEditor::GetSelectedNode() are declared very large block
they are not used and/or referred. So, we can get rid of some variables or
move smaller block.
For making each diff compact, this bug needs some patches.
First of all, this patch moves implementation of
nsIHTMLEditor::GetSelectedElement() to new non-virtual method
HTMLEditor::GetSelectedNode().
The HTMLEditor::GetSelectedElement() is ugly. Probably, it's checked only with
expected simple cases since the result does not make sense in some cases.
For example, when Selection is collapsed, it returns an element only when
"href" is specified with the argument. When Selection selects only one
element node (including its children), it quickly returns the node, however,
in the slow path, it returns second element node if first element node matches
with the argument. Or returns first element ndoe if it does not match with
the argument.
For preventing regressions, new test is designed to keep current odd behavior.
The new test is disabled only debug build on Android because adding this test
causes permanent orange of non-related test,
dom/tests/mochitest/fetch/test_request.html, see bug 1480702.
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
nsIHTMLEditor::Indent() is used for handling Tab key in HandleKeyPressEvent()
and used for implementing indent/outdent commands. Unfortunately, it takes
string argument to switch between indent or outdent. So, it does not make
sense to use this in C++ code.
This patch creates IndentAsAction() and OutdentAsAction() as public methods
and the implementation is moved to IndentOrOutdentAsSubAction() which takes
EditSubAction to switch between indent and outdent.
Note that HandleKeyPressEvent() uses the new public methods. However, this
is not problem for the future changes since HandleKeyPressEvent() is an
exception which may call other public methods.
Differential Revision: https://phabricator.services.mozilla.com/D3202
--HG--
extra : moz-landing-system : lando
We need to make it possible nsIHTMLEditor::SetCaretAfterElement() to distinguish
if it's called by outer class or editor itself. Therefore, this patch creates
CollapseSelectionAfter() for internal use.
Differential Revision: https://phabricator.services.mozilla.com/D3188
--HG--
extra : moz-landing-system : lando
The new test is disabled only debug build on Android because adding this test
causes permanent orange of non-related test,
dom/tests/mochitest/fetch/test_request.html, see bug 1480702.
Differential Revision: https://phabricator.services.mozilla.com/D3187
--HG--
extra : moz-landing-system : lando
For making it possible HTMLEditor::SelectElement() to distinguish if it's
called by outer class or editor itself, this patch creates
HTMLEditor::SelectContentInternal().
Differential Revision: https://phabricator.services.mozilla.com/D3001
--HG--
extra : moz-landing-system : lando
The new test is disabled only debug build on Android because adding this test
causes permanent orange of non-related test,
dom/tests/mochitest/fetch/test_request.html, see bug 1480702.
Differential Revision: https://phabricator.services.mozilla.com/D3186
--HG--
extra : moz-landing-system : lando
Correctness improvements:
* UTF errors are handled safely per spec instead of dangerously truncating
strings.
* There are fewer converter implementations.
Performance improvements:
* The old code did exact buffer length math, which meant doing UTF math twice
on each input string (once for length calculation and another time for
conversion). Exact length math is more complicated when handling errors
properly, which the old code didn't do. The new code does UTF math on the
string content only once (when converting) but risks allocating more than
once. There are heuristics in place to lower the probability of
reallocation in cases where the double math avoidance isn't enough of a
saving to absorb an allocation and memcpy.
* Previously, in UTF-16 <-> UTF-8 conversions, an ASCII prefix was optimized
but a single non-ASCII code point pessimized the rest of the string. The
new code tries to get back on the fast ASCII path.
* UTF-16 to Latin1 conversion guarantees less about handling of out-of-range
input to eliminate an operation from the inner loop on x86/x86_64.
* When assigning to a pre-existing string, the new code tries to reuse the
old buffer instead of first releasing the old buffer and then allocating a
new one.
* When reallocating from the new code, the memcpy covers only the data that
is part of the logical length of the old string instead of memcpying the
whole capacity. (For old callers old excess memcpy behavior is preserved
due to bogus callers. See bug 1472113.)
* UTF-8 strings in XPConnect that are in the Latin1 range are passed to
SpiderMonkey as Latin1.
New features:
* Conversion between UTF-8 and Latin1 is added in order to enable faster
future interop between Rust code (or otherwise UTF-8-using code) and text
node and SpiderMonkey code that uses Latin1.
MozReview-Commit-ID: JaJuExfILM9
For making it possible HTMLEditor::SelectElement() to distinguish if it's
called by outer class or editor itself, this patch creates
HTMLEditor::SelectContentInternal().
Differential Revision: https://phabricator.services.mozilla.com/D3001
--HG--
extra : moz-landing-system : lando
For making it possible HTMLEditor::SelectElement() to distinguish if it's
called by outer class or editor itself, this patch creates
HTMLEditor::SelectContentInternal().
Differential Revision: https://phabricator.services.mozilla.com/D3001
--HG--
extra : moz-landing-system : lando
For making it possible to distinguish if HTMLEditor::RemoveInlineProperty() is
called by outer class or editor itself, this patch creates
Create HTMLEditor::RemoveInlinePropertyInternal() and makes the internal
callers use this new method.
Differential Revision: https://phabricator.services.mozilla.com/D3000
--HG--
extra : moz-landing-system : lando
For making it possible to distinguish if SetInlineProperty() is called by outer
class or the editor itself, this patch creates SetInlinePropertyInternal().
Additionally, this makes the first argument of SetInlineProperty() from
nsAtom* to nsAtom& since it's not nullable.
Differential Revision: https://phabricator.services.mozilla.com/D2999
--HG--
extra : moz-landing-system : lando
Dorel Luca
Makoto Kato
Masayuki Nakano
Daniel Varga
Noemi Erli
Ehsan Akhgari
Cosmin Sabou
Andreea Pavel
Henri Sivonen
Tiberius Oros
Adrian Wielgosik