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
Unfortunately, TextServicesDocument::InsertText() is too complicated to
do it with both editor class and TextServicesDocument separately.
Therefore, this patch adds AutoTransactionBatchExternal class which is
almost same as AutoTransactionBatch but uses XPCOM methods to begin/end
transaction. This change helps editor to manage whether it starts to
handle new edit action or not when BeginTransaction() is called explicitly.
Differential Revision: https://phabricator.services.mozilla.com/D9478
--HG--
extra : moz-landing-system : lando
StyleUpdatingCommand::ToggleState() removes exclusive style when it sets
superscript style or subscript style. However, it accesses protected
members of EditorBase via AutoTransactionBatch even though it's outside
of editor classes.
This patch moves the removing exclusive style code from the method to
HTMLEditor::SetInlineProperty() and rename it "AsAction".
Differential Revision: https://phabricator.services.mozilla.com/D9477
--HG--
extra : moz-landing-system : lando
Depends on D8739.
This changeset updates calls to ok() that were most likely intended
for is(), but are not working as is.
Differential Revision: https://phabricator.services.mozilla.com/D8740
--HG--
extra : moz-landing-system : lando
This changeset updates all the test that were wrongly using ok() and wanted to
use is() AND for which the assert is still passing without any modification
required.
Differential Revision: https://phabricator.services.mozilla.com/D8739
--HG--
extra : moz-landing-system : lando
A lot of loops in HTMLTableEditor.cpp scans cells along column-axis or
row-axis. In those cases, they need to skip columns/rows which are spanned
from prior column/row. So, we can rewrite them with CellData::NextColumnIndex()
or CellData::NextRowColumn().
Differential Revision: https://phabricator.services.mozilla.com/D8358
--HG--
extra : moz-landing-system : lando
This patch makes all nsITableEditor::GetCellDataAt() use CellData, and if
each caller checks the result of GetCellDataAt() with NS_FAILED(), this
replaces it with CellData::FailedOrNotFound() since GetCellDataAt() returns
error even when it does not find a cell element. Finally, copies each
CellData member to the variable which received corresponding value from
GetCellDataAt() for making this change safe. Note that for easier to review,
the copying blocks have odd indent. Those variables will be removed or
corrected the indent by the following patches.
Differential Revision: https://phabricator.services.mozilla.com/D8339
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetCellDataAt() is an XPCOM method but used internally a lot.
Additionally, it scatters a lot of variables (including unused) since it takes
a lot of out arguments to return. Therefore, this should be implemented as
struct and users should refer each member as result.
This does not replaces the callers yet since it's too risky if the caller is
not tested by automated test. Following patches will replace the method call
and each variable step by step.
Differential Revision: https://phabricator.services.mozilla.com/D8338
--HG--
extra : moz-landing-system : lando
This patch makes EventStateManager handle middle click paste as a default
action.
Unfortunately, we cannot remove the call of HandleMiddleClickPaste() in
EditorEventListener because it's important to consume middle click event
before any elements in the editor. For example, if clicked HTMLEditor has
non-editable <a href> element, middle click event needs to be handled by the
editor rather than contentAreaUtils which handles click events of <a href>
elements. The cause of this kind of issues is, any click event handlers
which handle non-primary button events still listen to "click" events.
Therefore, this patch makes HandleMiddleClickPaste() do nothing if the mouseup
event is fired on an editor.
Differential Revision: https://phabricator.services.mozilla.com/D7855
--HG--
extra : moz-landing-system : lando
This is preparation of the last patch. Even if no editor is clicked with
middle button, we need to do:
- collapse Selection at the clicked point.
- dispatch "paste" event.
Therefore, HandleMiddleClickPaste() should dispatch ePaste event by itself
and each editor methods should have a bool argument which the caller wants
ePaste event automatically.
Note that Chromium dispatches "paste" event and pastes clipboard content
into clicked editor even if preceding "auxclick" event is consumed.
However, our traditional behavior is not dispatching "paste" event nor
pasting clipboard content. Unless Chromium developer keeps their odd
behavior, we should keep our traditional behavior since our behavior is
conforming to DOM event model.
Differential Revision: https://phabricator.services.mozilla.com/D7854
--HG--
extra : moz-landing-system : lando
The event argument of only EditorEventListener::MouseClick() can be replaced
with WidgetMouseEvent simply. So, for avoiding unnecessary RefPtr in
EditorEventListener::HandleEvent(), we should fix this now.
Differential Revision: https://phabricator.services.mozilla.com/D7853
--HG--
extra : moz-landing-system : lando
EventStateManager needs to handle middle click paste without editor.
Therefore, the handler should be in EventStateManager.
Differential Revision: https://phabricator.services.mozilla.com/D7852
--HG--
extra : moz-landing-system : lando
We need to move EditorEventListener::HandleMiddleClickPaste() into
EventStateManager to handle middle click paste after all click events are
dispatched. This is preparation of the change.
HandleMiddleClickPaste() uses UIEvent::GetRangeParent() and
UIEvent::RangeOffset() to collapse Selection at clicked point. However,
EventStateManager cannot access them since EventStateManager can handle it
with WidgetMouseEvent. Fortunately, only WidgetMouseEvent is necessary for
implementing them. Therefore, we can move the implementation into
nsLayoutUtils and merge them.
Differential Revision: https://phabricator.services.mozilla.com/D7851
--HG--
extra : moz-landing-system : lando
The ancestor limiter is set by focus event handler of editor. But window that
is run script has no focus, this event isn't fired by addRange etc.
Some execCommand's commands such as 'forwardDelete' uses WillDeleteSelection
then selection for deletion is set by selection controller (in
TextEditor::ExtendSelectionForDelete). So, due to no ancestor limiter, caret
(and any for delete commands such as CharacterExtendForDelete) can move to out
of editor's root.
So we should set ancestor limiter if nothing. If focus event is received by
user interaction etc, limiter will be set again.
Differential Revision: https://phabricator.services.mozilla.com/D6374
--HG--
extra : rebase_source : 240c3bc09b37d46d1ce3245bcca55cafc59454c5
FindSelectionRoot isn't const method and returns already_AddRefed<nsIContent>.
But this method doesn't modify any members and nodes, so we can change to const
method
Also, this method already returns Element, so it shouldn't return nsIContent.
Differential Revision: https://phabricator.services.mozilla.com/D6373
--HG--
extra : rebase_source : d4a5dbe96dfc0a71b39f3d5c6d1a4c7ce85f4fa9
If class A is derived from class B, then an instance of class A can be
converted to B via a static cast, so a slower QI is not needed.
Differential Revision: https://phabricator.services.mozilla.com/D6861
--HG--
extra : moz-landing-system : lando
nsITableEditor::InsertTableCell() is an XPCOM method but used internally. So,
HTMLEditor should implement it with a non-virtual method and all internal users
should use it instead.
Differential Revision: https://phabricator.services.mozilla.com/D6259
--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
HTMLEditor::DeleteTableCellWithTransaction() does not remove <table> element
even if it becomes empty only when 2 or more cell elements are selected.
Therefore, we should check table size before removing a row and if it's the
last row, the method should remove <table>.
Differential Revision: https://phabricator.services.mozilla.com/D6177
--HG--
extra : moz-landing-system : lando
nsITableEditor::DeleteTableCell() is an XPCOM method but used internally.
So, HTMLEditor should implement it with non-virtual method and use it
internally.
Differential Revision: https://phabricator.services.mozilla.com/D6176
--HG--
extra : moz-landing-system : lando
nsITableEditor::DeleteTableCellContents() is an XPCOM method but it's used
internally. So, HTMLEditor should implement it with a non-virtual method
and all internal users should use the non-virtual method instead.
This patch adds HTMLEditor::DeleteTableCellContentsWithTransaction() for that.
Additionally, this patch renames its helper method DeleteCellContents() to
DeleteAllChidlrenWithTransaction() and moves it to HTMLEditor.cpp since it can
handle any element nodes.
Differential Revision: https://phabricator.services.mozilla.com/D6174
--HG--
extra : moz-landing-system : lando
This patch renames HTMLEditor::DeleteColumn() to
HTMLEditor::DeleteTableColumnWithTransaction() and cleans up its implementation.
Differential Revision: https://phabricator.services.mozilla.com/D5934
--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
This add automated tests for nsITableEditor.deleteTableColumn().
However, this contains some fixes of existing code since with those bugs,
the test isn't passed even in the simplest case.
Differential Revision: https://phabricator.services.mozilla.com/D5932
--HG--
extra : moz-landing-system : lando
This patch renames HTMLEditor::DeleteRow() to
HTMLEditor::DeleteTableRowWithTransaction() and cleans up its implementation.
Differential Revision: https://phabricator.services.mozilla.com/D5931
--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
This patch changes a bit in HTMLEditor::DeleteTableRow() because calling
DeleteTable2() without those helper classes hits MOZ_ASSERT().
Differential Revision: https://phabricator.services.mozilla.com/D5929
--HG--
extra : moz-landing-system : lando
Since I have landed bug 1489939, comm-central only uses rewrap method in
nsIEditorMailSupport. And insertAsCitedQuotation is used by the test of
editor/libeditor/tests/test_bug616590.xul.
Other methods are unused now, so let's remove these from nsIEditorMailSupprt,
and move it to HTMLEditor. Also, pasteAsQuotation is unused now even if
BlueGriffon.
Differential Revision: https://phabricator.services.mozilla.com/D5835
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetSelectedOrParentTableElement() is of course an XPCOM method,
but it's used internally. So, there should be non-virtual method.
On the other hand, this API is too ugly. Returns various information but each
meaning is not clear. So, result of the new non-virtual method should be
simpler.
This patch creates the new method as:
- returns found element
- takes ErrorResult to return error code.
- takes optional out-param for returning if a cell is selected.
Then, nsITableEditor::GetSelectedOrParentTableElement() can compute its
result from them with Selection.
Differential Revision: https://phabricator.services.mozilla.com/D5728
--HG--
extra : moz-landing-system : lando
If a cell element and its indexes in the <table> are stored in a struct,
it makes the user methods easier to read. Therefore, this patch implement
such struct as HTMLEditor::CellAndIndexes and make it finds first selected
cell and indexes with its method. Finally, this reimplement
HTMLEditor::GetFirstSelectedCellInTable() with it.
Differential Revision: https://phabricator.services.mozilla.com/D5664
--HG--
extra : moz-landing-system : lando
This adds automated tests for nsITableEditor.getFirstSelectedCellInTable().
However, this test crashes due to regression of bug 1484128. So, we need
to uplift only this fix into the Beta.
Differential Revision: https://phabricator.services.mozilla.com/D5663
--HG--
extra : moz-landing-system : lando
I tried to create automated tests for nsITableEditor::NormalizeTable().
However, this method cannot normalize any broken table element. The method
always returns error after calling HTMLEditor::FixBadRowSpan(). The reason
is that HTMLEditor::FixBadRowSpan() scans all cells in each row with a for
loop, and then, when it fails to find a cell element, it returns error even
though this method needs to fix the odd situation. According to the history
of editor changes, each important point hasn't been changed since first
implementation. So, perhaps, table layout API behavior was changed but
no automated tests couldn't detect the regression since we really don't have
enough tests for editor.
Anyway, this patch makes most part of nsITableEditor::NormalizeTable() with
non-virtual method, HTMLEditor::NormalizeTable().
Differential Revision: https://phabricator.services.mozilla.com/D5633
--HG--
extra : moz-landing-system : lando
When I added some tests into test_middle_click_paste.html, I realized that
SimpleTest.waitForClipboard() in copyHTMLContent() fails to copy the
HTML fragment to clipboard and just quit the test. Therefore, only the
last tests are ignored always.
The reason is, iframe.contentDocument.getSelection() returns nullptr
since the frame becomes visible immediately before accessing the Selection.
This patch makes flushing the pending layout with scrollTop. This makes
getSelection() return non-null.
However, unfortunately, only on Linux, it fails to copy the content. I'm
still not sure the reason. This patch just avoids running the last part
only on Linux.
Differential Revision: https://phabricator.services.mozilla.com/D5742
--HG--
extra : moz-landing-system : lando
Although this reftest is sometimes failure on Android, it is no meaning to run
this test on Android due to no spellchekcer on Firefox Android.
Differential Revision: https://phabricator.services.mozilla.com/D5745
--HG--
extra : moz-landing-system : lando
This enables the editor directory to be linted, and fixes the remaining issues raised by ESLint. Various rules were fixed here including, no-shadow, no-undef, no-unused-vars and others.
I've generally gone conservative, disabling rules where it doesn't make sense to fix them (e.g. sometimes suggests use-services for tests, but it is only used once, or within a Chrome script).
Depends on D5585
Differential Revision: https://phabricator.services.mozilla.com/D5587
--HG--
extra : moz-landing-system : lando
These are all automatically generated by ESLint with the --fix option.
Depends on D5584
Differential Revision: https://phabricator.services.mozilla.com/D5585
--HG--
extra : moz-landing-system : lando
Also block-disables no-multi-spaces for test_resizers_resizing_elements.html
Differential Revision: https://phabricator.services.mozilla.com/D5584
--HG--
extra : moz-landing-system : lando
HTMLEditor::RefereshGrabber() is an XPCOM method which is used by BlueGriffon.
Additionally, it's called internally. Therefore, we should create a non-virtual
method for this and all internal users should use it.
This patch renames all other related methods to *Internal() for consistency.
Additionally, this fixes a bug of nested calls of ShowGrabber() and
HideGrabber(). This makes CreateGrabber() sets mGrabber directly since
it may be cleared by HideGrabber() while it's running, and also makes
HideGrabber() moves all members who will be cleaned up with local variables
and always clean them up even if it meats an error.
Differential Revision: https://phabricator.services.mozilla.com/D5424
--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
nsIHTMLObjectResizers.resizedObject is used only for avoiding warning of
nsIHTMLObjectResizers.refreshResizers() if resizers are not visible.
Therefore, if we remove the unnecessary warnings, we can get rid of the
attribute.
Differential Revision: https://phabricator.services.mozilla.com/D5427
--HG--
extra : moz-landing-system : lando
HTMLEditor::RefreshResizers() is an XPCOM method but it's used internally. So,
HTMLEditor should implement it with non-virtual method and use new one for
internal use.
This patch also makes related methods nested-creation of resizers aware. This
issue must not be dangerous, but looks like buggy.
Differential Revision: https://phabricator.services.mozilla.com/D5426
--HG--
extra : moz-landing-system : lando
Since I have landed bug 1478546, no one (inc. bluegriffon) uses this method.
Differential Revision: https://phabricator.services.mozilla.com/D5497
--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, nobody uses nsIHTMLObjectResizer.showResizers(). So, we can remove it.
Then, we have two private methods, one is non-virtual
HTMLEditor::ShowResizers(), and the other is HTMLEditor::ShowResizersInner().
HTMLEditor::ShowResizers() calls HTMLEditor::ShowResizersInner() and if
it fails, calls HideResizers(). However, in some cases, e.g., when it already
has visible resizers, hiding resizers does not make sense.
So, this patch removes non-virtual HTMLEditor::ShowResizers() method too,
then, renames HTMLEditor::ShowResizersInner() to HTMLEditor::ShowResizers(),
finally, it hides resizers only when it fails to setup resizers for keeping
resizers hidden.
Differential Revision: https://phabricator.services.mozilla.com/D4921
--HG--
extra : moz-landing-system : lando
Because of the rewrite of test_request.html, now, we can enable the mochitests
for editor which were disabled on debug build of Android due to bug 1480702.
Differential Revision: https://phabricator.services.mozilla.com/D4904
--HG--
extra : moz-landing-system : lando
Summary:
Now HTMLEditor::IsInLink returns boolean and nsINode if returning true.
I would like to return Element instead of boolean for reducing refcounting
and QI like HTMLEditRules::IsInListItem.
Reviewers: masayuki
Tags: #secure-revision
Bug #: 1487305
Differential Revision: https://phabricator.services.mozilla.com/D4640
--HG--
extra : rebase_source : 275629d7e1108b4e56fb39fa745f491bfb55963d
Even though HTMLEditor::GetIsCSSEnabled() is an implementation of an XPCOM
method, it uses nsresult as its return type. We should change it to
NS_IMETHODIMP.
Additionally, SetDocumentStateCommand::GetCommandStateParams() calls this,
but HTMLEditor can expose non-virtual method, HTMLEditor::IsCSSEnabled().
Therefore, this patch makes it public and makes SetDocumentStateCommand use
HTMLEditor::IsCSSEnabled().
Differential Revision: https://phabricator.services.mozilla.com/D4304
--HG--
extra : moz-landing-system : lando
Neither comm-central nor BlueGriffon uses nsIHTMLEditor.getFontColorSetate().
So, we can get rid of this from nsIHTMLEditor. However, we need to keep it
as a non-virtual public method since it's used by FontColorStateCommand.
Differential Revision: https://phabricator.services.mozilla.com/D4303
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetNextCellElement() is an XPCOM method but it's used internally
a lot. So, HTMLEditor should implement it with non-virtual method and
internal users should use the non-virtual method.
Therefore, this patch creates HTMLEditor::GetNextSelectedTableCellElement().
Differential Revision: https://phabricator.services.mozilla.com/D4194
--HG--
extra : moz-landing-system : lando
There are surprisingly many of them.
(Plus a couple of unnecessary checks after `new` calls that were nearby.)
--HG--
extra : rebase_source : 47b6d5d7c5c99b1b50b396daf7a3b67abfd74fc1
Oddly, on 63 Beta simulation, nsIDocument::GetWindow() may return nullptr
when HTMLEditor is being destroyed by unload of the page. I'm not sure if
this is an expected change. However, HTMLEditor::HideResizers() should
not stop cleaning up even if it meets unexpected situation.
Additionally, this patch moves all HTMLEditor members related to resizers
to local variables since while HideResizers() is cleaning up old resizers,
the members may be overwritten by ShowResizers() if mutation event listener
or something does something.
Differential Revision: https://phabricator.services.mozilla.com/D4057
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetFirstSelectedCell() is an XPCOM method, but used internally a
lot. Therefore, we should create a non-virtual method for internal use.
This patch creates HTMLEditor::GetFirstSelectedTableCellElement(), and it
won't return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND since nobody needs the
value. It's enough to check whether the result is nullptr without error for
any callers.
Differential Revision: https://phabricator.services.mozilla.com/D4060
--HG--
extra : moz-landing-system : lando
Oddly, on Android, size of resized objects may be 2px different from ideal
value. I don't know the reason, could be zoom level or something is affected.
However, fortunately, this difference is not important for this test because
this test checks whether resizers actually works with specific elements.
So, even if the result is 2px smaller or bigger than ideal value, we succeeded
to check the resizer makes the element bigger or smaller as expected.
Therefore, this patch makes the test allow 2px differences of the result.
Additionally, on Android, this test is always timed out if TV (even opt build).
So, this patch disables TV on Android.
Differential Revision: https://phabricator.services.mozilla.com/D4058
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetCellAt() is an XPCOM method, but this is called internally
a lot. So, we should create non-virtual method for internal use.
The XPCOM method retrieves a <table> element if given element is nullptr.
However, no internal user needs this feature. So, we can create
GetTableCellElementAt() simply. Then, we can get rid of nsresult and
ErrorResult from it.
Differential Revision: https://phabricator.services.mozilla.com/D3956
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetTableSize() is an XPCOM method but used internally a lot.
Therefore, it shouldn't be called for internal use. Additionally, the
callers need to declare two int32_t variables, but this causes the code
messy. Therefore, this patch creates TableSize struct and it implements
HTMLEditor::GetTableSize(). Then, all callers of it is replaced with
TableSize struct.
New TableSize struct does not support computes <table> element from anchor
of Selection since there is no user of this in C++ code.
Differential Revision: https://phabricator.services.mozilla.com/D3953
--HG--
extra : moz-landing-system : lando
HTMLEditor::GetCellIndexes() is an XPCOM method and used a lot internally.
So, we need alternative way to retrieve indexes of a cell without virtual
calls. In a lot of places, receiving indexes with 2 int32_t variables causes
the code messy and that causes making it harder to understand which are
index for same cell and where they come from. So, making both of them stored
one variable makes the callers simpler. Therefore, this patch creates
CellIndexes stack class to get and store the result simply. Then, this makes
all callers of GetCellIndexes() use this new class and makes GetCellIndexes()
also use this new class.
FYI: This patch does NOT put ErrorResult instances in small block scope as far as
possible. The reason is, I see its destructor in profile sometimes. I don't think
that we should use nsresult& instead of ErrorResult& only for this performance
reason, but I think that creating each instance in loops does not make sense.
Differential Revision: https://phabricator.services.mozilla.com/D3849
--HG--
extra : moz-landing-system : lando
Nobody uses nsITableEditor.getNextRow(). Therefore, this patch removes this
XPCOM API.
Differential Revision: https://phabricator.services.mozilla.com/D3949
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetNextRow() is an XPCOM method. Therefore, we should have
a non-virtual method for internal use of it.
This changes the definition in nsITableEditor. First, it allows only <tr>
element as what HTMLEditor::GetNextRow() has actually done. Then, changes
the return type to Element since it always returns an element node.
Differential Revision: https://phabricator.services.mozilla.com/D3948
--HG--
extra : moz-landing-system : lando
There are 2 bugs:
One is a simple mistake. kTest is each item of the tests, kTests is array of
all tests. When it needs to refer kTest.isAbsolutePosition, it referred
kTests.isAbsolutePosiiton. Therefore, the test always failed to enable
editing UI for absolute positioned element.
The other is, this test requires to disable inline-table-editing UI (which is
add or remove rows and columns). Note that even if the UI is disabled,
resizers is available for <table> elements.
Differential Revision: https://phabricator.services.mozilla.com/D3954
--HG--
extra : moz-landing-system : lando
nsITableEditor::GetFirstRow() is an XPCOM method, so, for internal use,
we should create non-virtual method, that is GetFirstTableRowElement().
This patch makes it never return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND since
nobody refers it and it's detectable. If the method returns nullptr without
error, it's the case of NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND.
Additionally, this patch changes the return type of GetFirstRow() from
Node to Element since it always return an Element node if not null.
Differential Revision: https://phabricator.services.mozilla.com/D3780
--HG--
extra : moz-landing-system : lando
Android's reftest of Bug 1443902 is failed rarely (Bug 1476914, Bug 1475049
Bug 1477502 and Bug 1476129) due to "image comparison, max difference: 1,
number of differing pixels: 1". But I don't know why difference is 1 and
I think that this might be reftest framework for Android or Android emulator
issue.
So I would like to add fuzzy-if as workaround.
Differential Revision: https://phabricator.services.mozilla.com/D3859
--HG--
extra : moz-landing-system : lando
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
User may paste a lot with pressing Accel+V for a while (i.e., with auto repeat).
So, calling nsIEditor::Paste() may be in a hot path and we can now make
non-virtual public method with AsHTMLEditor().
Differential Revision: https://phabricator.services.mozilla.com/D2993
--HG--
extra : moz-landing-system : lando
HTMLEditor::Paste() is an override of nsIEditor. So, it's virtual and public.
We should use protected method for internal use and should make it non-virtual
if possible. This patch creates PasteInternal() which is a protected
non-virtual method.
Differential Revision: https://phabricator.services.mozilla.com/D2992
--HG--
extra : moz-landing-system : lando
SplitStyleAbovePoint calls SplitNodeDeepWithTransaction repeatedly. If
SplitNodeDeepWithTransaction creates orphan node like this test case,
this crash occurs. So we should check whether node becomes orphan node.
Differential Revision: https://phabricator.services.mozilla.com/D1993
--HG--
extra : moz-landing-system : lando
This patch was written entirely by the following script:
#!/bin/bash
if [ ! -d "./.hg" ]
then
echo "Not in a source tree." 1>&2
exit 1
fi
find . -regex '.*\(ref\|crash\)test.*\.list' | while read FILENAME
do
echo "Processing ${FILENAME}."
# The following has four substitutions:
# * The first one replaces the *first* argument to fuzzy() when it doesn't
# have a - in it, by replacing it with an explicit 0-N range.
# * The second one does the same for the *second* argument to fuzzy().
# * The third does the same for the *second* argument to fuzzy-if().
# * The fourth does the same for the *third* argument to fuzzy-if().
#
# Note that this is using perl rather than sed because perl doesn't
# support non-greedy matching, which is needed for the first argument to
# fuzzy-if.
perl -pi -e 's/(fuzzy\()([^ ,()-]*)(,[^ ,()]*\))/${1}0-${2}${3}/g;s/(fuzzy\([^ ,()]*,)([^ ,()-]*)(\))/${1}0-${2}${3}/g;s/(fuzzy-if\([^ ]*?,)([^ ,()-]*)(,[^ ,()]*\))/${1}0-${2}${3}/g;s/(fuzzy-if\([^ ]*?,[^ ,()]*,)([^ ,()-]*)(\))/${1}0-${2}${3}/g' "${FILENAME}"
done
Differential Revision: https://phabricator.services.mozilla.com/D2974
--HG--
extra : moz-landing-system : lando
HTMLEditor::InsertTextWithQuotations() is called by HTMLEditor::Rewrap() but
it's an XPCOM method, i.e., can be called by anybody. For making possible to
distinguish whether it's called by outer or not, we should create non-virtual
method and Rewrap() should use it instead.
Differential Revision: https://phabricator.services.mozilla.com/D2991
--HG--
extra : moz-landing-system : lando
This patch also removes LOCK_DOC() and UNLOCK_DOC() from TextServiceDocument.cpp
since they looks like we cannot use early-return style without call of
UNLOCK_DOC() even after removing EndTransaction() calls. However, they
are defined as empty. So, they do nothing even since first landing.
Therefore, there is no reason to keep them.
Differential Revision: https://phabricator.services.mozilla.com/D2990
--HG--
extra : moz-landing-system : lando
This patch also creates non-virtual methods,
EditorBase::BeginTransactionInternal(), EditorBase::EndTransactionInternal(),
TransactionManager::BeginBatchInternal() and
TransactionManager::EndBatchInternal().
Although, this could be replaced with API to use PlaceholderTransaction,
we should investigate it when we have much time.
Differential Revision: https://phabricator.services.mozilla.com/D2989
--HG--
extra : moz-landing-system : lando
The regressing bug made RepaintSelectionRunner do nothing for any
nsISelectionListener that wasn't a PresShell.
Differential Revision: https://phabricator.services.mozilla.com/D2846
--HG--
extra : moz-landing-system : lando
Although HTMLEditor::EndUpdateViewBatch() calls a method of nsIHTMLEditor,
the additional work after calling EditorBase::EndUpdateViewBatch() is enough
simple. So, we can move the implementation in HTMLEditor to EditorBase and
make it non-virtual.
Differential Revision: https://phabricator.services.mozilla.com/D2705
--HG--
extra : moz-landing-system : lando
