`PresShell.h` is exposed as `mozilla/PresShell.h` and `PresShell` is the only
concrete class of `nsIPresShell`. Therefore, we have no reason to access
`PresShell` via `nsIPresShell`.
Differential Revision: https://phabricator.services.mozilla.com/D23277
--HG--
extra : moz-landing-system : lando
As far as I've tested, Chrome does not throw exception even when editor is
destroyed or editor content is modified unexpectedly. So, we should return
`NS_OK` from most public methods of editor when internal methods return
`NS_ERROR_EDITOR_DESTROYED` or `NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE`.
Differential Revision: https://phabricator.services.mozilla.com/D20811
--HG--
extra : moz-landing-system : lando
Those probes are now expired and we got enough data:
- Almost no user uses the grip to move absolute positioned element
- There were over one thousand users using the inline table editor and the object resizers.
- Such users keep using even after we disabled the UIs by default.
Perhaps, such small number of users keep using the UIs, i.e., I guess the
number won't become smaller in short term. Therefore, this patch removes the
telemetry probes and members of HTMLEditor which are necessary to call
Telemetry API.
Differential Revision: https://phabricator.services.mozilla.com/D20609
--HG--
extra : moz-landing-system : lando
nsITableEditor::InsertTableColumn() is an XPCOM method but it's used internally.
So, HTMLEditor should implement it with a non-virtual method and internal
users should use it instead.
Differential Revision: https://phabricator.services.mozilla.com/D6257
--HG--
extra : moz-landing-system : lando
nsITableEditor::InsertTableRow() is an XPCOM method but there are some internal
users. So, HTMLEditor should implement it with a non-virtual method and
it should be used by all internal users.
Differential Revision: https://phabricator.services.mozilla.com/D6179
--HG--
extra : moz-landing-system : lando
nsITableEditor::DeleteTableColumn() is an XPCOM method but used internally.
So, it should be implemented with non-virtual method and internal users
should use it.
Note that this changes only one thing. This moves
|AutoTopLevelEditSubActionNotifier maybeTopLevelEditSubAction(...| from
below DeleteTableElementAndChildrenWithTransaction() to above it.
I.e., DeleteTableElementAndChildrenWithTransaction() works under
EditSubAction::eDeleteNode as the top level sub-action now. This is same
as DeleteSelectedTableRowsWithTransaction(). Therefore, the difference
with it when it removes <table> is now fixed.
Differential Revision: https://phabricator.services.mozilla.com/D5933
--HG--
extra : moz-landing-system : lando
nsITableEditor::DeleteTableRow() is an XPCOM method but there are some internal
users. So, it should be implemented as non-virtual protected method and
internal users should use it instead.
This also renames (and reimplement) HTMLEditor::DeleteTable2() since it's
really bad name and the code dispatches unnecessary "selectionchange" events.
Differential Revision: https://phabricator.services.mozilla.com/D5930
--HG--
extra : moz-landing-system : lando
HTMLEditor::RefreshInlineTableEditingUI() is an XPCOM method. Therefore,
we should create a non-virtual method for internal use.
Additionally, this patch makes related methods safer for nested calls of
ShowInlineTableEditingUI() and HideInlineTableEditingUI(). If
ShowInlineTableEditingUI() and RefreshInlineTableEditingUIInternal() detects
hiding or replacing current UI, they return error to make the callers stop
handling anything for new UI.
Differential Revision: https://phabricator.services.mozilla.com/D5428
--HG--
extra : moz-landing-system : lando
HTMLEditor::HideResizers() is an XPCOM method, so, we should create non-virtual
method for internal use.
Differential Revision: https://phabricator.services.mozilla.com/D4923
--HG--
extra : moz-landing-system : lando
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.
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
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
Gecko has some built-in UIs:
* to edit size of objects like <img>, <table> and absolute positioned elements.
* to edit position of absolute positioned elements.
* to add/remove table columns and rows.
Currently, those UIs are available in both designMode editor and contenteditable
editor only on Gecko. I.e., the other browsers' users cannot modify as such
without web apps implement such function. So, for compatibility with the
other browsers, we should hide those UIs by default. On the other hand, if
this is too risky for backward compatibility, we should not do that.
So, before doing that, we should collect actual usage data of object resizers,
inline table editing UI, positioning UI of absolute positioned elements with
telemetry probes.
This patch adds 3 sets of probes for each UI. One is percentage of showing
each UI in all instantiated HTMLEditor. The other is number of user interaction
of each UI in HTMLEditors which has shown the UI.
This patch makes all new probes as "opt-out" because they are really important
data since used for deciding whether those UIs are necessary or unnecessary.
MozReview-Commit-ID: B9Y6GTiCPw6
--HG--
extra : rebase_source : 00e49f31712e24cb269ad3aa65c7d13b7cccb3a5
Also switch the XPCOM-y version of EventTarget::AddEventListner to a
Nullable<bool> for aWantsUntrusted.
The three-arg overload of AddEventListener in ContentFrameMessageManager was
never called, so all the AddEventListener overloads there are not needed.
MozReview-Commit-ID: 4IhqHmPVWzE
We can't have a null content in
ScrollbarActivity::StopListeningForScrollAreaEvents, because only viewport
frames have a null GetContent().
MozReview-Commit-ID: 9iAg0ivVqqG
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
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
inlineTableEditingEnabled and refreshInlineTableEditingUI are used by
bluegriffon, but other methods aren't used by m-c, c-c and bluegriffon.
So I would like to remove these methods. Then we can clean up
DoInlineTableEditingAction.
MozReview-Commit-ID: 3R0bJDU5vqv
--HG--
extra : rebase_source : 74c7615613bba65326069929cb53c3ea48f2e9a4
extra : amend_source : a1ac9af3de50daa0cbfd98e2790afa402d9932ef
mInlineEditedCell for inline table editor is still nsIDOMElement. To avoid QI
and virtual call, we should replace it with Element.
MozReview-Commit-ID: 76yfQJxwm9d
--HG--
extra : rebase_source : e89de17faa20d16a8823e1016b6ba8a6f78993b6
CreateAnonymousElement still uses nsIDOMNode, but we should use nsIContent for it.
MozReview-Commit-ID: 2xgzlE6NVra
--HG--
extra : rebase_source : c93a9acb230604da391f705b0038e3cf7fbd4f23
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
Resizer and etc attributes on table editor still use nsIDOMElement. Converting to Element makes both implementation and the callers simpler.
MozReview-Commit-ID: TTFSvqn5GE
--HG--
extra : rebase_source : 705576c4eb0fe5f8f566f3415a8a72842c919edd
In our coding rules, variable names of nsresult should be rv. Indeed, when you see |rv| in the code, you must assume that its type if nsresult.
However, a lot of code under editor/ uses |res| for the variables of nsresult. Let's replace |res| with |rv|.
And this patch improves following points:
1. When |rv| is set in both |if| and |else| block and they are check outside of them, this moves the check into each |if| and |else| block because even if the failure is notified with warning, you cannot see which case was performed and failed. This change makes it clear.
2. When |return rv;| returns non-error code because |rv| is checked with NS_ENSURE_SUCCESS() immediately before, setting replacing it with |return NS_OK;| is clearer.
3. Move declaration of |nsresult rv| into smaller scope as far as possible. This prevents setting rv to unexpected value and easier to check its value at reading the code.
MozReview-Commit-ID: 9MAqj7sFey3
--HG--
extra : rebase_source : 0fd316b851ea616b3a95d8c1afc111ff55e11993
Masayuki Nakano
Sylvestre Ledru
Emilio Cobos Álvarez
Boris Zbarsky
Makoto Kato
Masatoshi Kimura
Bobby Holley
Cameron McCormack