Some methods in `nsIEditingSession` isn't used from script. So we should move
these to `nsEditingSession` or add `[noscript]`.
Differential Revision: https://phabricator.services.mozilla.com/D33606
--HG--
extra : moz-landing-system : lando
Having NAC bound to the tree when not connected is not quite fine, make sure to
clean up properly.
Differential Revision: https://phabricator.services.mozilla.com/D33704
--HG--
extra : moz-landing-system : lando
`HTMLEditor` initializes selection ancestor limit when it receives `focus`
event. If `Document.execCommand()` is called immediately after an
ancestor of active editing host becomes new editing host,
`HTMLEditor::GetActiveEditingHost()` returns the new one, but selection
ancestor limit is still the previous one. This mismatch causes a lot of
bugs. Therefore, this patch makes `nsGenericHTMLElement` notifies `HTMLEditor`
of an element becoming `contenteditable`, and makes `HTMLEditor` update
selection ancestor limit only when the new editing host is ancestor of
old selection ancestor limit.
Differential Revision: https://phabricator.services.mozilla.com/D32823
--HG--
extra : moz-landing-system : lando
`cmd_align` is always with `nsCommandParams` when it's executed. However,
when somebody checks whether the command is enabled or not, or retrieves the
state, `GetInternalCommand()` is called without `nsCommandParams`. Therefore,
even when `nsCommandParmas` is nullptr for `cmd_align`, `GetInternalCommand()`
shouldn't warn it.
Additionally, internal command supports to set `align` to empty string.
Therefore, `GetInternalCommand()` also needs to support it.
This patch adds `Command::FormatJustify` for the former case and
`Command::FormatJustifyNone` for the latter case.
Note that this does not affect to actual behavior since `AlignCommand`
does not refer the result of `GetInternalCommand()`.
Differential Revision: https://phabricator.services.mozilla.com/D33604
--HG--
extra : moz-landing-system : lando
BindContext was going to have way more information at first, but then I realized
that most of the things I wanted to know were basically a flag away using the
parent node.
Still I think it's worth it, now experimenting with BindToTree will only mean
adding a field to a struct that's included from a couple cpp files, instead of a
massive pain.
I also think this is clearer, and doing this highlights quite a few
inconsistencies in our code which I've left untouched, but commented with
FIXMEs.
Steps are:
$ for file in $(rg 'nsresult BindToTree\(' | cut -d : -f 1 | sort | uniq); do sed -i 's#nsresult BindToTree(Document\* aDocument, nsIContent\* aParent,#nsresult BindToTree(BindContext\&, nsINode\& aParent)#g' $file; done
$ for file in $(rg 'nsresult BindToTree\(' | cut -d : -f 1 | sort | uniq); do sed -i 's# nsIContent\* aBindingParent) override#override#g' $file; done
$ for file in $(rg '::BindToTree\(' | cut -d : -f 1 | sort | uniq); do sed -i 's#::BindToTree(Document\* aDocument, nsIContent\* aParent,#::BindToTree(BindContext\& aContext, nsINode\& aParent)#g' $file; done
$ for file in $(rg '::BindToTree\(' | cut -d : -f 1 | sort | uniq); do sed -i 's#nsIContent\* aBindingParent)##g' $file; done
$ for file in $(rg '::BindToTree\(' | cut -d : -f 1 | sort | uniq); do sed -i 's#::BindToTree(aDocument, aParent, aBindingParent)#::BindToTree(aContext, aParent)#g' $file; done
$ ./mach clang-format
Then manual fixups.
Depends on D32948
Differential Revision: https://phabricator.services.mozilla.com/D32949
I think this is a good change regardless of other discussion in bug 1552587. If
we decide to move `mColor` to the top-level of the struct that can be done
separately.
Differential Revision: https://phabricator.services.mozilla.com/D32726
--HG--
extra : moz-landing-system : lando
`aPointAfterInsertedString` of `WSRunObject::InsertText()` is optional (i.e.,
may be nullptr). However, the fix for bug 1534394 makes it always set to
original insertion point when `InsertTextWithTransaction()` is failed.
This patch just make it check if `aPointAfterInsertedString` is nullptr before
setting its value.
Differential Revision: https://phabricator.services.mozilla.com/D33144
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::ApplyBlockStyle()` stores `curBlock` and `newBlock` during its
loop to keep handling from deeper child to ancestor, and may do two things for
a `curNode`. If `curBlock` and/or `newBlock` is moved from expected container
when it sets one of or both of them, this patch check whether mutation event
listeners change the DOM tree. Additionally, this patch also checks whether
`curNode' is moved by mutation event listener at first step of two jobs for it.
Differential Revision: https://phabricator.services.mozilla.com/D32689
--HG--
extra : moz-landing-system : lando
`WSRunObject` scans previous and next node of given point/range **without**
checking editing host boundary. Therefore, its methods may return non-editable
nodes or editable nodes in another editing host. In such cases, `HTMLEditRules`
is confused.
This patch makes it store editing host at initialization and it check the
boundary. However, the former cost may appear in score of some benchmark
tests, but we shouldn't allow attackers to use this entrance.
Differential Revision: https://phabricator.services.mozilla.com/D32467
--HG--
extra : moz-landing-system : lando
Currently, `nsISelectionDisplay::DISPLAY_ALL` is used only by `HTMLEditor`.
And only when it's set, `nsImageFrame::ShouldDisplaySelection()` returns `false`
if only its `mContent` is selected. However, this is based on an assumption,
that is, when only one `<img>` is selected in an HTML editor, it's target of
resizers. However, this is completely wrong. Web apps can disable resizers
with `document.execCommand("enableObjectResizing", false, false)` and now,
it's disabled by default.
Therefore, this patch makes the method check whether its `mContent` is
target of resizers at the moment.
Differential Revision: https://phabricator.services.mozilla.com/D32449
--HG--
extra : moz-landing-system : lando
test_CF_HTML_clipboard.html does nothing if platform isn't mac. But according
to intermittent failure log, this is often failure on Android.
I guess that this is infra issue, but we should use skip-if to avoid this
failure instead.
Differential Revision: https://phabricator.services.mozilla.com/D32413
--HG--
extra : moz-landing-system : lando
Oddly, `WSRunObject::InsertText()` returns `NS_OK` even when
`HTMLEditor::InsertTextWithTransaction()` returns error. However,
it fails if insertion point is not editable like `<noscript>` element.
In such case, `aPointAfterInsertedString` isn't modified and its caller,
`HTMLEditRules::WillInsertText()` keep handling inserting remaining text
with non-positioned `EditorDOMPoint`. Therefore, at the next time,
`WSRunObject` fails to do anything since it requires positioned
`EditorDOMPoint`.
For making uplift safer, this patch makes `WSRunObject::InsertText()` set
`aPointAfterInsertedString` by itself when
`HTMLEditor::InsertTextWithTransaction()` returns error.
Differential Revision: https://phabricator.services.mozilla.com/D32131
--HG--
extra : moz-landing-system : lando
`GetInternalCommand()` is currently used only by `EditorCommand` and it
treats the additional parameter only when given command is `cmd_align`.
However, the value is complicated since `AlignCommand` allows both `CString`
value and `String` value. Therefore, `EditorCommand::DoCommandParams()` may
fail to solve `cmd_align` to a `Command` value without checking both of them.
Therefore, it must make sense that `GetInternalCommand()` take `nsCommandParams`
as optional argument and check it only when given command matches `cmd_align`.
Then, we don't need to waste unnecessary run-time cost.
Note that this bug has been hidden since `AlignCommand` class does not refer
the `Command` value but refers only `nsCommandParams`. However, the previous
patch makes `EditorCommand::GetParamType()` not allow `Command::DoNothing`.
Therefore, we need this follow-up fix now.
Differential Revision: https://phabricator.services.mozilla.com/D30501
--HG--
extra : moz-landing-system : lando
If `nsIControllerCommand::DoCommandParams()` is called without aParams or
`nsITransferable` pointer, this patch sets nullptr to `aTransferableParam` for
`DoCommandParam()`. This allows each implementation to choose ignore or
return error.
Differential Revision: https://phabricator.services.mozilla.com/D30500
--HG--
extra : moz-landing-system : lando
Only `MultiStateCommandBase::DoCommandParams()` allows `CString` param and
`String` param (the former is preferred). This patch makes
`EditorCommand::DoCommandParams()` aware of this case.
Differential Revision: https://phabricator.services.mozilla.com/D30499
--HG--
extra : moz-landing-system : lando
If `nsIControllerCommand::DoCommandParams()` is called with `nullptr` for its
`aParams`, this patch sets `VoidString()` to `DoCommandParam()` for making
each implementation be able to consider whether the case is an error or
treat it as specific default value.
Differential Revision: https://phabricator.services.mozilla.com/D30498
--HG--
extra : moz-landing-system : lando
If `nsIControllerCommand::DoCommandParams()` is called with `nullptr` for its
`aParams`, this patch sets `VoidCString()` to `DoCommandParam()` for making
each implementation be able to consider whether the case is an error or
treat it as specific default value.
Differential Revision: https://phabricator.services.mozilla.com/D30497
--HG--
extra : moz-landing-system : lando
We should use `Maybe` for `bool` because some command may treat the default
value when the parameter is omitted as `true` or `false. Although, current
implementation does not do that.
Differential Revision: https://phabricator.services.mozilla.com/D30496
--HG--
extra : moz-landing-system : lando
Most `EditorCommand` classes don't require additional params for executing
a command. All of them just calls their `DoCommand()` or returns same result.
So, we can create new virtual method,
`EditorCommand::DoCommandParam(Command aCommand, TextEditor& aTextEditor)`,
which just delegates to `DoCommand()`.
This patch adds some undeclared commands but which are handled by
`EditorCommand` subclasses, and changes `CommandInt` type from `int8_t` to
`uint8_t` since the count of `Command` items becomes over 128.
Differential Revision: https://phabricator.services.mozilla.com/D30495
--HG--
extra : moz-landing-system : lando
`nsIDocumentStateListener` is a scriptable interface and each method may run
any script. So, we should mark them as `can_run_script`. Then, we need to
mark a lot of editing methods because we need to mark
`EditorBase::EndTransactionInternal()` and `EditorBase::DoTransactionInternal()`
as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D30360
--HG--
extra : moz-landing-system : lando
This seems to be regression by bug 1362858.
Actually, single quotation mark is always separator for spellchecker after
landing bug 1462858. When user tries to input "doesn't", "'" becomes separator
for spellchecker. Then "doesn" will be misspell word.
So we shouldn't mark single quotation mark as separator if user is inputting
word.
Differential Revision: https://phabricator.services.mozilla.com/D29153
--HG--
extra : moz-landing-system : lando
It'd be better to change copy constructor of `EditorDOMPointBase` to explicit,
but it'd require too many changes in editor code. So, this patch just changes
each method callers only.
Differential Revision: https://phabricator.services.mozilla.com/D30054
--HG--
extra : moz-landing-system : lando