DragEvent::GetDataTransfer() is fallible when memory allocation fails as it lazily constructs DataTransfer. This tries failing gracely when that happens.
Differential Revision: https://phabricator.services.mozilla.com/D97663
There are not enough tests comparing delete operation result and result of
`getTargetRanges()` when selection is in/around/across list item elements.
This patch creates a utility method to make the test body not need to use
Selection API for making simpler tests.
The expected behavior is based on Blink and WebKit unless their behavior is
buggy because their behavior is more reasonable than Gecko's in most cases.
Note that the removing tests are covered by the new tests.
Differential Revision: https://phabricator.services.mozilla.com/D96800
I guess that `DataTransfer::HasType()` is inlined in the opt builds and
actually crashed in `EditorEventListener::DragEventHasSupportingData()`
at accessing `aDragEvent->GetDataTransfer()` result without null-check
because `DataTransfer::mItems` is set to `nullptr` only by the cycle
collector, but it does not make sense to think that it occurs the STR
in bug 1627673 comment 3.
Therefore, this patch adds null-checks in
`EditorEventListener::DragEventHasSupportingData()`.
I have no idea how to test this with automated tests.
Differential Revision: https://phabricator.services.mozilla.com/D96310
`InsertBRElementIfHardLineIsEmptyAndEndsWithBlockBoundary()` checks whether
a line containing `aPointToInsert` requires invisible `<br>` element.
Therefore, `aPointToInsert` must be set and valid. However, all selection
ranges may be removed by somebody when modifying the DOM tree before that.
Therefore, this patch makes all callers of it check whether they succeeded
to retrieve caret position or not.
Differential Revision: https://phabricator.services.mozilla.com/D96280
They join first line of a descendant block element into the line at the other
block contains the descendant block. First of all of their jobs, they call
`DeleteInvisibleASCIIWhiteSpace()` to clean up unnecessary whitespaces at
the other block. At this time, the point which tells whether the descendant
element is contained in the other block may become invalid if script runs
and it causes changing the DOM tree. Therefore, they should check the
given point is still valid after calling `DeleteInvisibleASCIIWhiteSpace()`
because `AutoTrackDOMPoint` requires valid point.
Differential Revision: https://phabricator.services.mozilla.com/D95987
This supports one manifestparser expression per line in the 'skip-if',
'fail-if' and 'run-if' keys. As a side effect the:
skip-if = foo ||
bar
syntax is no longer supported. Instead it can be:
skip-if =
foo # bug 123
bar # bug 456
Differential Revision: https://phabricator.services.mozilla.com/D95927
`HTMLEditor` ignores input events when a text control element in an editing
host has focus.
https://searchfox.org/mozilla-central/rev/d866b96d74ec2a63f09ee418f048d23f4fd379a2/editor/libeditor/HTMLEditor.cpp#6322
However, focus event does not check it and modifies ancestor limiter of
`Selection` for the document. And even in this case, public methods for
handling some edit actions are open for `execCommand` requests. Then,
edit action handlers may set selection outside the elements, but new focus
event to update the selection ancestor limiter will be handled later.
Therefore, until getting next focus event, `HTMLEditor` does not work as
expected since `Selection::*InLimiter()` APIs refuse to change selection.
Ideally, we should make `nsFocusManager` notifies editor of focus change
synchronously, but I cannot fix it quickly because it needs to touch security
sensitive area. Therefore, this patch makes `HTMLEditor` do not set
ancestor limiter when a text control in any editing host gets focus.
Additionally, this patch makes
`HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::HandleDeleteAtOtherBlockBoundary`
stop handling leaf node deletion if it cannot change caret position.
Differential Revision: https://phabricator.services.mozilla.com/D95268
The test runs `execCommand("insertHorizontalRule")` when the document node
has no children. In this case, `TextFragmentData` fails to initialize itself.
In this case, our editor cannot do anything. Therefore, returning error
in this case must be better.
This patch makes all callers of the scan methods handle the error case unless
the caller cannot return error.
Comparing with Blink, perhaps, we should insert `<html>` and `<body>` element
in this case first and keep doing the requested operation in the future.
Currently, doing it may cause another complicated issue with mutation event
listeners.
Differential Revision: https://phabricator.services.mozilla.com/D95126
The reason of hitting the assertion is, a clipboard event listener nests
another edit action, initializing the editor, runs, and tries to create an
anonymous `<br>` element with transaction, but it checks whether `beforeinput`
event has already been dispatched or not with the parent edit action data.
For fixing it, this patch adds a flag to edit action data to indicate whether
a clipboard event has already been dispatched or not. If it's already been
dispatched, it's **okay** to modify the DOM tree. Ideally, we should put off
modifying the DOM tree after dispatching `beforeinput` event, but this case
does not notify web apps anything so that this patch should be fine.
Although this patch adds a crash test which is attached by the reporter,
it's not reproducible with current editor code because we stopped modifying
native anonymous nodes with transaction, but it depends on the behavior.
Still this assertion hits in `test_clipboard_noeditor.html` intermittently,
but I don't have idea how to reproduce this permanently. Therefore, this
patch does not contain a test to check to regression actually.
Differential Revision: https://phabricator.services.mozilla.com/D95115
Actually, `DeleteNodeWithTransaction` cannot remove non-editable node. But
we should allow it which parent node is ediatble.
Differential Revision: https://phabricator.services.mozilla.com/D94937
It's too big (+15k lines) and there are 2 groups which does not depend on the
others. One is getting current state for `Document.queryCommandState` etc.
This is copied to `HTMLEditorState.cpp`. And the other is delete handlers.
This is copied to `HTMLEditorDeleteHandler.cpp`.
Differential Revision: https://phabricator.services.mozilla.com/D94934
When `beforeinput` event is canceled, the editor content shouldn't be modified.
Therefore, `runTest()` can handle it with caching initial value before
running the test body.
Additionally, this patch makes every test ensure that target editor has focus.
So, finally, each test is now isolated completely. Developers can run only
a test with commenting out the other tests.
Depends on D94520
Differential Revision: https://phabricator.services.mozilla.com/D94521
This patch makes all test functions call `runTest()` and it adds event listeners
only while running the function. Therefore, this makes each test function do:
* initializing for running test
* running test with `runTest` and specifying expected data
Depends on D94519
Differential Revision: https://phabricator.services.mozilla.com/D94520
This allows the following patches will make all functions use batch function
to test with a framework.
Depends on D94517
Differential Revision: https://phabricator.services.mozilla.com/D94518
So, this removes functions named as `*_and_canceling_beforeinput` and
makes corresponding function take an object which has `action` and
`cancelBeforeInput` to reduce dependency on wider scope variables.
Depends on D94516
Differential Revision: https://phabricator.services.mozilla.com/D94517
Although, still there are dependencies between the functions, this allows
stack has meaningful name where a failure occurs.
Depends on D94515
Differential Revision: https://phabricator.services.mozilla.com/D94516
Indent level of the tests will be increased. Therefore, all descriptions
should be in their own lines.
Note that this and following patches rewrite almost all lines in the test
files again and again. Therefore, it's hard to rebase and existing bugs
will be fixed when I was writing each patch. So, I'd like reviewers just
put inline comment which should be fixed before landing. Then, I'll add
a patch to fix them.
Differential Revision: https://phabricator.services.mozilla.com/D94515
So, the root cause of this assertion hit was, I forgot to add this path
when I implement the methods to compute target ranges. Although the other
browsers don't support multiple selection ranges, this patch adds WPTs
for the cases.
Depends on D94241
Differential Revision: https://phabricator.services.mozilla.com/D94242
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
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
This is a simple mistake. If the given range is collapsed, scanning text node
to before/after from collapsed range may create reversed range.
Differential Revision: https://phabricator.services.mozilla.com/D94225
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
Simon Giesecke
Kagami Sascha Rosylight
Mirko Brodesser
Masayuki Nakano
Andrew Halberstadt
tobar2018
Makoto Kato
Ricky Stewart
Edgar Chen
Bogdan Tara
Henri Sivonen
Mihai Alexandru Michis