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
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
This patch renames HTMLEditor::DeleteRow() to
HTMLEditor::DeleteTableRowWithTransaction() and cleans up its implementation.
Differential Revision: https://phabricator.services.mozilla.com/D5931
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
It's always created with non-nullptr. So, making it treat reference of
EditorBase makes its implementation simpler.
Note that this changes comment in EditorBase::InsertTextWithTransaction() to
a MOZ_ASSERT() for detecting bugs. However, the comment is wrong. When
the insertion is for updating composition string, callers don't need to create
AutoTransactionsConserveSelection since it'll be ignored by
CompositionTransaction. So, the new MOZ_ASSERT() checks whether it's
in composition or prevented transaction changing Selection.
MozReview-Commit-ID: 6jZ4LpksyoD
TextEditor::DeleteSelectionAsAction() is called even if it's a part of edit
action. For example, it's called to prepare for inserting text.
For bug 1465702, editor itself and edit rules classes should not call
public DeleteSelectionAsAction() directly. Therefore, this patch creates
DeleteSelectionAsSubAction() for internal use.
Note that this patch adds NS_ASSERTION() to detect wrong caller. However,
it cannot distinguish if the call is valid, for example, it's allowed to
call DeleteSelectionAsSelection() even if it's handling an edit action but
the method is called via mutation event listener. So, we need to allow
some assertions with some tests. But unfortunately, 1405747.html uses
mutation event listener too many times (about 1,000 times) and the number
of assertion isn't stable. Therefore, this patch makes the test stop using
the mutation event listener 2nd time since I can reproduce the crash with
ESR 52 at the 2nd time.
MozReview-Commit-ID: 1TWaypmnoCC
When we implement InputEvent.inputType, we need to set a stack class to record
which edit action is currently handled. However, currently, we call smaller
jobs as edit action. For example, when user types a character at selecting
some characters, then, EditAction::deleteSelection is performed first, then,
EditAction::insertText is performed. However, for the InputEvent.inputType,
we need inserText information. So, for making new enum EditAction, we need
to rename current EditAction to EditSubAction.
And also this renames related stuff:
EditorBase::mIsInEditAction -> EditorBase::mIsInEditSubAction
EditorBase::IsInEditAction() -> EditorBase::IsInEditSubAction()
EditorBase::mAction -> EditorBase::mTopLevelEditSubAction
TextEditRules::mTheAction -> TextEditRules::mTopLevelEditSubAction
EditorBase::StartOperation() ->
EditorBase::OnStartToHandleTopLevelEditSubAction()
EditorBase::EndOperation() ->
EditorBase::OnEndHandlingTopLevelEditSubAction()
AutoRules -> AutoTopLevelEditSubActionNotifier
RulesInfo -> EditSubActionInfo
MozReview-Commit-ID: cvSkPUjFm1
The patch is a bit large because all these methods except
SswitchTableCellHeaderType call each other; doing it piecemeal would have
required introducing, then removing, a bunch of QIs.
The parameters of EditorBase::ReplaceContainer() are complicated. For example,
if it's specified as cloning all attributes, aAttribute and aValue are ignored
because CloneAttributes() removes all existing attributes but ReplaceContainer()
sets attributes before calling CloneAttributes(). This method has 3 modes:
1. Just replaces aOldContainer with new element.
2. #1 and clones all attributes from aOldContainer to the new element.
3. #1 and sets aAttribute of the new element to aValue.
Therefore, this patch creates 3 inline wrappers of the renamed method.
MozReview-Commit-ID: IsPu2uZuU8f
First, EditorBase::DeleteSelection() is never used since
TextEditor::DeleteSelection() overrides it but does not call it. So, this patch
makes EditorBase::DeleteSelection() only returns NS_ERROR_NOT_IMPLEMENTED.
Next, EditorBase::DeleteSelectionImpl() actually removes content for
TextEditor::DeleteSelection(). So, it should be named as
DeleteSelectionWithTransaction(). However, it'll be done in the following
patch. On the other hand, its callers are EditorBase::HandleKeyPressEvent()
and EditorBase::DeleteSelectionAndPrepareToCreateNode(). Fortunately, they
can be moved to TextEditor simply. Therefore this patch moves the methods
to TextEditor for making related methods in a place.
Then, we can make the implementation of nsIEditor::TextEditor() as a non-virtual
method, TextEditor::DeleteSelectionAsAction().
MozReview-Commit-ID: KXFDhW3G9lA
A lot of methods take |const EditorRawDOMPoint&| as their argument. However,
some of them are called with EditorDOMPoint::AsRaw(). This is not good for
performance because:
1. Needs to create temporary instance of EditorRawDOMPoint.
2. EditorRawDOMPoint::AsRaw() may be used by simple mistake.
3. Such methods may call EditorRawDOMPoint::Offset(), however, it's not copied
to the original EditorDOMPoint instance. So, callers may need to compute
offset again.
So, such methods should be changed to template methods if they are not virtual
method and AsRaw() should be gotten rid of for prevent it looking not expensive.
MozReview-Commit-ID: DAqqW4a2EMS
Some methods in HTMLEditor uses NS_IMETHOD that doesn't override. So we should
replace with nsresult to avoid virtual method.
MozReview-Commit-ID: 7LillLQNhlg
Masayuki Nakano