It's hard to understand each caller of `HTMLEditor::IsEmptyNode()` tries to
check with multiple `bool` arguments. Therefore, they should be replaced
with an `EnumSet`.
Note that only the first argument is reverted the meaning. Therefore, if
it's omitted or `false`, `EmptyCheckOption::TreatSingleBRElementAsVisible`
is specified explicitly. Otherwise, i.e., `true`, nothing should be
specified.
Differential Revision: https://phabricator.services.mozilla.com/D112513
The lifetime of it is guaranteed by `AutoEditActionDataSetter` which grabs
`Selection` until it's destroyed, and it's a stack only class and created
at first step of all public method calls. Therefore, we can mark it as
`MOZ_KNOWN_LIVE` and we can change it returning reference of `Selection`
instead of reference of `RefPtr<Selection>`.
Differential Revision: https://phabricator.services.mozilla.com/D110896
The method is tested by `test_nsITableEditor_getFirstSelectedCellInTable.html`.
And now, nobody uses `CellAndIndexes` so that this patch removes it.
Depends on D94239
Differential Revision: https://phabricator.services.mozilla.com/D94240
Unfortunately, there is no automated tests for `nsITableEditor.joinTableCells()`
and it'll take a couple of days to write its testcase because of too big
method. And it's not so important for Firefox (although it's used in
comm-central). Therefore, this patch skips to check it.
Note that `CellAndIndexes` uses `HTMLEditor::GetFirstSelectedTableCellElement()`
to retrieve first selected cell element and then, it retrieves its indexes.
https://searchfox.org/mozilla-central/rev/61728de8273c04fe2417c475fc0637e8b79210d7/editor/libeditor/HTMLTableEditor.cpp#4090,4098,4109
Therefore, this patch makes it stop using this simple structure too.
Depends on D94235
Differential Revision: https://phabricator.services.mozilla.com/D94236
These APIs are used only in comm-central (not in BlueGriffon) and nobody
refers the out argument which is selected range of the cell selection.
Therefore, we should drop them to make the APIs simpler.
Differential Revision: https://phabricator.services.mozilla.com/D94226
These APIs are used only in comm-central (not in BlueGriffon) and nobody
refers the out argument which is selected range of the cell selection.
Therefore, we should drop them to make the APIs simpler.
Differential Revision: https://phabricator.services.mozilla.com/D94226
The root cause of this bug is a bug of async `ScrollSelectionIntoView` that
is what it won't be canceled even if web app changes scroll position with
any API. Fixing it is really risky and this bug affects an existing website.
Therefore, this patch makes the constructors of `AutoPlaceholderBatch` take
whether do or do not `ScrollSelectionIntoView` when it's destructed. And
makes `HTMLEditor::SetInlinePropertyAsAction()` not request
`ScrollSelectionIntoView` for taking back the traditional behavior.
Note that this patch does not make it an optional argument because it's hard to
guess that it'll run `ScrollSelectionIntoView` automatically.
Differential Revision: https://phabricator.services.mozilla.com/D87819
The location wasn't used from the caller of
`GetTableCellLocationFromRange`.
However, `GetTableCellLocationFromRange`
included flushing frames, this is now done in
`HTMLEditor::CellIndexes::Update`.
Differential Revision: https://phabricator.services.mozilla.com/D75098
Even though the method returns only in specific cases, but the result affects
only one caller, `HTMLEditor::GetNextSelectedTableCellElement()`. Therefore,
we can create new generic utility method,
`HTMLEditUtils::GetTableCellElementIfOnlyOneSelected()` and get rid of
`HTMLEditor::GetCellFromRange()`.
Note that the warnings in `HTMLEditor::GetCellFromRange()` is just noise for
any callers. So, this gets rid of the useless spam warnings.
Differential Revision: https://phabricator.services.mozilla.com/D72586
As an alternative of `HTMLEditor::GetCellFromRange()`, this patch creates more
generic utility method.
Depends on D72296
Differential Revision: https://phabricator.services.mozilla.com/D72585
`DeleteSelection*()` are members of `TextEditor`, but they are also used by
`HTMLEditor`. Therefore, they and pref cache members for them should be
in `EditorBase` too.
Depends on D71911
Differential Revision: https://phabricator.services.mozilla.com/D72290
Their users should use `EditorDOMPoint` or something instead.
This patch cleans up `EditorBase::DoJoinNodes()` too because of their heavy
user. It requires only joined nodes because `aParent` is used only for
removing `aContentToJoin`, but we now have `nsINode::Remove()` which does
not require parent node and never fails.
Depends on D70877
Differential Revision: https://phabricator.services.mozilla.com/D70878
We can relax about `GetElementOrParentElement*()` because they just return
`nullptr` when selection anchor is a `Document` node.
Additionally, this patch renames the internal APIs to the names similar to
modern DOM API.
Finally, this adds automated test for
`nsIHTMLEditor.getElementOrParentByTagName()`.
Differential Revision: https://phabricator.services.mozilla.com/D70152
It's valid thing that a container of a `Range` of `Selection` is not a content
node. Actually, it can be a `Document` node. But it's illegal case for
editor. So, if `HTMLEditor` meets such case, it does not need to do anything.
This patch makes that if `HTMLEditor` meets the situation at very first time
of public edit action method, it returns "OK" for avoiding new exception case.
Otherwise, i.e., it's an XPCOM API or meeting such situation after a DOM
mutation, returns error.
Differential Revision: https://phabricator.services.mozilla.com/D65278
It's valid thing that a container of a `Range` of `Selection` is not a content
node. Actually, it can be a `Document` node. But it's illegal case for
editor. So, if `HTMLEditor` meets such case, it does not need to do anything.
This patch makes that if `HTMLEditor` meets the situation at very first time
of public edit action method, it returns "OK" for avoiding new exception case.
Otherwise, i.e., it's an XPCOM API or meeting such situation after a DOM
mutation, returns error.
Differential Revision: https://phabricator.services.mozilla.com/D65278
IsEmptyNode won't return error if parameter is not null. So we shouldn't use
nserror for return value and use assertion to check parameter.
Differential Revision: https://phabricator.services.mozilla.com/D63124
Previously, I added `Selection::mCachedRange` to save allocation cost of
`nsRange`. However, with the previous patch, we don't need the hack anymore
since ranges removed by `Selection::RemoveAllRanges()` are always kept in
the global cache of `nsRange`. Therefore, we can remove the ugly hack right
now.
Differential Revision: https://phabricator.services.mozilla.com/D61239
`nsRange` instances are allocated a lot in the heap especially by editor and
spellchecker. The allocation cost is too bad for benchmarks. Therefore,
we should reuse released instances as far as possible. For managing it in
static factory methods of `nsRange`, we need to hide `nsRange` constructor.
Differential Revision: https://phabricator.services.mozilla.com/D61237
* `nsIHTMLEditor.removeAllInlineProperties`
* `nsIHTMLEditor.increaseFontSize`
* `nsIHTMLEditor.decreaseFontSize`
* `nsIHTMLEditor.setParagraphFormat`
* `nsIHTMLEditor.getBackgroundColorState`
* `nsIHTMLEditor.indent`
* `nsIHTMLEditor.align`
* `nsIEditorStyleSheets.replaceOverrideStyleSheet`
* `nsITableEditor.selectBlockOfCells`
These methods are not used by any Gecko products including comm-central and
BlueGriffon so that we should remove them. Note that only
`HTMLEditor::GetBackgroundColorState()` is used internally so that we need to
keep it as a public method of `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D61139
* getAttributeValue
* cloneAttribute
* splitNode
* joinNodes
* markNodeDirty
* removeEditorObserver
are not used from script. Therefore, we can get rid of them from `nsIEditor`.
However, `EditorBase::GetAttributeValue()` is referred by
`HTMLEditor::CopyCellBackgroundColor()` and it's just a wrapper of
`Element::GetAttr()`. Therefore, this patch makes
`HTMLEditor::CopyCellBackgroundColor()` use `Element::GetAttr()` directly.
And also `EditorBase::MarkNodeDirty()` is used from some friend classes.
Therefore, this patch redesigns it as `MarkElementDirty()` and make all of
them check whether the method call destroys the editor.
Differential Revision: https://phabricator.services.mozilla.com/D60796
`AutoEditActionDataSetter` is created in the stack when editor's public method
is called and that guarantees lifetime of global objects in editor such as
editor itself, selection controller, etc.
The dispatcher of `beforeinput` event returns `NS_ERROR_EDITOR_ACTION_CANCELED`
if an event is actually dispatched but canceled. The reason why it's an error
is, editor code must stop handling anything when any methods return error.
So, returning an error code is reasonable in editor module. But when it's
filtered by `EditorBase::ToGenericNSResult()` at return statement of public
methods, it's converted to `NS_SUCCESS_DOM_NO_OPERATION`. This avoids throwing
new exception, but editor class users in C++ can distinguish whether each edit
action is canceled or handled. The reason why we should not throw new
exception from XPCOM API is, without taking care of each caller may break some
our UI (especially for avoiding to break comm-central). Therefore, this patch
does not make XPCOM methods return error code when `beforeinput` event is
canceled.
In most cases, immediately after creating `AutoEditActionDataSetter` is good
timing to dispatch `beforeinput` event since editor has not touched the DOM
yet. If `beforeinput` requires `data` or `dataTransfer`, methods need to
dispatch `beforeinput` event after that. Alhtough this is not a good thing
from point of view of consistency of the code. However, I have no better
idea.
Note 1: Our implementation does NOT conform to the spec about event order
between `keypress` and `beforeinput` (dispatching `beforeinput` event after
`keypress` event). However, we follow all other browsers' behavior so that it
must be safe and the spec should be updated for backward compatibility.
Spec issue: https://github.com/w3c/uievents/issues/220
Note 2: Our implementation does NOT conform to the spec about event order
between `compositionupdate` and `beforeinput`. Our behavior is same as
Safari, but different from Chrome. This might cause web-compat issues.
However, our behavior does make sense from point of view of consistency of
event spec. Additionally, at both `compositionupdate` and `beforeinput`,
composition string in editor has not been modified yet. Therefore, this
may not cause web-compat issues (and I hope so).
Spec issue: https://github.com/w3c/input-events/issues/49
Note that this patch makes editor detect bugs that `beforeinput` event hasn't
been handled yet when it dispatches `input` event or modifying `data` and
`dataTransfer` value are modified after dispatching `beforeinput` event with
`MOZ_ASSERT`s.
Differential Revision: https://phabricator.services.mozilla.com/D58127
`AutoEditActionDataSetter` is created in the stack when editor's public method
is called and that guarantees lifetime of global objects in editor such as
editor itself, selection controller, etc.
The dispatcher of `beforeinput` event returns `NS_ERROR_EDITOR_ACTION_CANCELED`
if an event is actually dispatched but canceled. The reason why it's an error
is, editor code must stop handling anything when any methods return error.
So, returning an error code is reasonable in editor module. But when it's
filtered by `EditorBase::ToGenericNSResult()` at return statement of public
methods, it's converted to `NS_SUCCESS_DOM_NO_OPERATION`. This avoids throwing
new exception, but editor class users in C++ can distinguish whether each edit
action is canceled or handled. The reason why we should not throw new
exception from XPCOM API is, without taking care of each caller may break some
our UI (especially for avoiding to break comm-central). Therefore, this patch
does not make XPCOM methods return error code when `beforeinput` event is
canceled.
In most cases, immediately after creating `AutoEditActionDataSetter` is good
timing to dispatch `beforeinput` event since editor has not touched the DOM
yet. If `beforeinput` requires `data` or `dataTransfer`, methods need to
dispatch `beforeinput` event after that. Alhtough this is not a good thing
from point of view of consistency of the code. However, I have no better
idea.
Note 1: Our implementation does NOT conform to the spec about event order
between `keypress` and `beforeinput` (dispatching `beforeinput` event after
`keypress` event). However, we follow all other browsers' behavior so that it
must be safe and the spec should be updated for backward compatibility.
Spec issue: https://github.com/w3c/uievents/issues/220
Note 2: Our implementation does NOT conform to the spec about event order
between `compositionupdate` and `beforeinput`. Our behavior is same as
Safari, but different from Chrome. This might cause web-compat issues.
However, our behavior does make sense from point of view of consistency of
event spec. Additionally, at both `compositionupdate` and `beforeinput`,
composition string in editor has not been modified yet. Therefore, this
may not cause web-compat issues (and I hope so).
Spec issue: https://github.com/w3c/input-events/issues/49
Note that this patch makes editor detect bugs that `beforeinput` event hasn't
been handled yet when it dispatches `input` event or modifying `data` and
`dataTransfer` value are modified after dispatching `beforeinput` event with
`MOZ_ASSERT`s.
Differential Revision: https://phabricator.services.mozilla.com/D58127
`uint32_t`, because `nsRange::ComparePoints` requires it -- by webidl
interface -- to be unsigned long.
Moreover it makes `RangeBoundaryBase`'s interface cleaner, because it
already exposes the offset as a `uint32_t`.
Differential Revision: https://phabricator.services.mozilla.com/D50054
And also this patch make each `AutoEditSubActionNotifier` creator check
the result of `OnStartToHandleTopLevelEditSubAction()` at least for
`NS_ERROR_EDITOR_DESTROYED`.
We need to take care of its destructor's result later, though.
Differential Revision: https://phabricator.services.mozilla.com/D45786
`TextEditor::DeleteSelectionAsSubAction()` starts to handle all
"delete selection" sub-actions (i.e., even if the instance is `HTMLEditor`).
Therefore, `TextEditRules::WillDeleteSelection()` should be renamed to
`TextEditor::HandleDeleteSelection()` and we need to make it virtual.
Differential Revision: https://phabricator.services.mozilla.com/D45300
The members of `HTMLEditRules` which are used only while `WillDoAction()` and
`DidDoAction()` are called should be moved to specific stack only struct
`EditorBase::EditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42106
Despite of their names, `TextEditRules::mCachedSelectionNode` and
`TextEditRules::mCachedSelectionOffset` are used only for calling
`EditorBase::HandleInlineSpellCheck()` so that they should be renamed to
explain the purpose.
Additionally, they are not necessary to be in the heap since they are
necessary until `TextEditRules::AfterEdit()` is called. Therefore, we can
move them into `EditorBase::HandleInlineSpellCheck()`.
Finally, `TextEditRules::BeforeEdit()` and `TextEditRules::AfterEdit()` are
called only by `TextEditor::OnStartToHandleTopLevelEditSubAction()` and
`TextEditor::OnEndHandlingTopLevelEditSubAction()`. Therefore, we can move
the setter to `TextEditor::OnStartToHandleTopLevelEditSubAction()`.
Differential Revision: https://phabricator.services.mozilla.com/D41396
Masayuki Nakano