Building with `ac_add_options --disable-unified-build` on macOS hits the following -Wunreachable-code-return warnings-as-errors:
editor/libeditor/HTMLEditor.cpp:4341:26 [-Wunreachable-code-return] 'return' will never be executed
editor/libeditor/HTMLTableEditor.cpp:1629:10: warning: 'return' will never be executed [-Wunreachable-code-return]
Differential Revision: https://phabricator.services.mozilla.com/D126441
Currently, `mozInlineSpellChecker` handles events in the default group. This
means that the listener may not run if web app stops the propagation. For
keeping the event listener order as far as possible, this patch changes to
listen to the event at capturing phase in the system group (editor handles
the events at bubbling phase in the system group).
Additionally, it listens to `keypress` events for handling caret/selection
changes. However, they should be handled at `keydown` because those keys
are not printable. Therefore, this patch changes to listen to `keydown`
events, but this could change the race between running the scheduled spellcheck
and following `keypress` event which is dispatched only for chrome code and
C++ event listeners. However, this may not change it actually because the
race is changed only when the following `keypress` event delays too much.
https://searchfox.org/mozilla-central/rev/2eebd6e256fa0355e08421265e57ee1307836d92/extensions/spellcheck/src/mozInlineSpellChecker.cpp#503-504
Differential Revision: https://phabricator.services.mozilla.com/D126635
The test requires to support allowing nested `execCommand` if we want to run
completely. From point of view of the purose of crashtests, we should allow
it for testing in more complicated situation. Therefore, this patch allows
it while the test is running by the pref.
Differential Revision: https://phabricator.services.mozilla.com/D125914
I have no idea how to make the test result stable because the intermittent
failure is caused by a race of `IMEContentObserver`'s content query performend
at vsync and `keypress` event after the `keydown` event. Therefore, I try to
fix the root cause of the failure.
There are two paths reaching a public method of editor with pending
notifications. One is `keypress` event listener of the editor. The other is
`execCommand` related methods of `Document`. Therefore, we should make them
flush pending notifications before accessing editor, but the cost may be too
expensive. So we should consider whether we should flush pending notifications
or not as far as later.
Note that we cannot fix the `beforeinput` cases because flushing pending
notifications after dispatching `beforeinput` event may cause oranges. It'll
be fixed while I'm working on bug 1710784.
Differential Revision: https://phabricator.services.mozilla.com/D125805
If we flush pending notifications before calling `execCommand` in
`insert-*-in-void-element.tentative.html`, `HTMLEditor` does not work because
`GetActiveEditingHost()` returns `nullptr`. This is wrong because `Selection`
cannot cross native anonymous subtree boundaries and the selection collapsed in
`<input>` element is valid. Therefore, it should not check whether the content
has independent selection or not.
Differential Revision: https://phabricator.services.mozilla.com/D125791
Although we don't have a testcase, `HTMLEditor` may need to delete a text node
if it's created for composition. If it's not allowed, users may see staying
composition string in disabled editor.
Differential Revision: https://phabricator.services.mozilla.com/D125502
* Rename arguments so that their names are consistent with Next().
* Make the function not assert on an empty string, i.e. aLen == 0, like
Next().
* Fix an undefined behavior when the user passes aTextLen == aOffset.
The methods used to access `aText[aOffset]` that is clearly out of range
because the string may not be null-terminated. After this patch, it
returns a sentinel WordRange when aLen == aPos.
* Add document and gtest TestFindWordWithEmptyString().
* Change the sentinel return value to {aLen,aLen} for FindWord(), and
adapt one caller.
Differential Revision: https://phabricator.services.mozilla.com/D125434
And this renames the method to `ExtendOrShrinkRangeToDelete` for alining to
new behavior changed by the following patch.
Depends on D125028
Differential Revision: https://phabricator.services.mozilla.com/D125029
Similar to the previous patch, this patch makes `insertLineBreak` command
handler of `HTMLEditor` may insert a linefeed character instead of `<br>`
element for compatibility with Blink/WebKit.
Note that for making same name rules as `insertParagraphSeparator` command
handlers, this patch renames
`HTMLEditor::InsertBRElementAtSelectionWithTransaction()` to
`InsertLineBreakAsSubAction()` and moves
`EditorBase::InsertLineBreakAsSubAction()` to `TextEditor` since it's
used only by `TextEditor` instance.
Depends on D124731
Differential Revision: https://phabricator.services.mozilla.com/D124732
Blink and WebKit inserts linefeed character for `insertParagraphSeparator`
command if:
* the insertion point's block parent is an editing host
* the editing host's outside display is "inline"
* the editing host's white space style preformats linefeed characters
For web-compat, we should do this in these conditions.
Depends on D124562
Differential Revision: https://phabricator.services.mozilla.com/D124731
This uses `setTimeout` for emulating original test as far as possible, but
unfortunately, we need to change `data` URI of the `<iframe>` to `srcdoc`
due to cross-origin error.
Differential Revision: https://phabricator.services.mozilla.com/D124846
This does NOT add tests to `inserttext` command which includes linefeed
characters with other characters. It seems that Blink handles 2 or more
length string is handled as multiple calls for every character. So each
linefeed character is handled as `insertparagraph`, but Gecko treats the
linefeed characters as-is. We should add the tests later with changing
Gecko's behavior, but for now, we should keep current behavior because
this difference must not be trobule in the wild, but we need to improve
preformatted string handler of Gecko better against reported web-compat
issues.
Differential Revision: https://phabricator.services.mozilla.com/D124562
`HTMLEditor` tries to collect all children in a line. However, it does not
handle preformatted linefeed characters. Especially when there is a
preformatted linefeed immediately before a block boundary, it becomes invisible.
So, the range of line needs to extend the range correctly similar to the case
if there is an invisible `<br>` element.
Differential Revision: https://phabricator.services.mozilla.com/D124561
So, the careful point is, they shouldn't put ASCII white-space next to a
preformatted white-space, but should put ASCII white-space next to the NBSP
as far as possible for making line break opportunities.
Differential Revision: https://phabricator.services.mozilla.com/D124560
If caret is put at preformatted linefeed, we need to handle adjacent
collapsible white-spaces too if the style is `white-space: pre-line`.
Therefore, this patch makes the delete handler aware of the case only
linefeed characters are preformatted and `AutoDeleteRangesHandler` call it
when caret is around a preformatted line break.
Differential Revision: https://phabricator.services.mozilla.com/D124559
It currently replaces inserting string's white-spaces to an `NBSP` even if
preformatted linefeed characters. Additionally, collapsible white-spaces around
a linefeed are removed. Therefore, adjacent collapsible characters of every
linefeed needs to be an NBSP. This patch makes the method handle it correctly.
Differential Revision: https://phabricator.services.mozilla.com/D124558
If `white-space` is `pre-line`, only linefeed characters are preformatted, but
white-spaces are collapsible. And if collapsible white-spaces follow or
are followed by a preformatted linefeed, they are removed. Therefore,
these methods need to scan following or preceding white-spaces of first
linefeed if the caller wants to delete. Unless doing it, the removed
white-spaces would become visible due to no linefeed character containing
sequence.
Differential Revision: https://phabricator.services.mozilla.com/D124557
Currently, they don't assume that there is `white-space: pre-line` which
preserves only linefeed characters, but white-spaces are collapsible.
This patch makes them stop early return when white-spaces are preformatted,
and do additional checks for linefeed characters.
Differential Revision: https://phabricator.services.mozilla.com/D124556
This makes it support `white-space: pre-line` which only preserve linefeed
characters. However, the APIs are not all of what we need to change.
Therefore, this causes hitting assertion hits due to different result from
some other APIs. So patching only with this does not work. The automated
test results will be updated later.
Note that it's **not** the goal of this bug that we don't support white-space
handling in text nodes whose `white-space` is `-moz-pre-space`. It replaces
linefeed characters with white-spaces then, preserve all white-spaces.
However, it's not standardized, and it's available with `xml:space="preserve"`
in SVG if web developers don't use the vender prefixed value, and finally,
supporting it makes our white-space handling in editor more complicated
than applying the following patches. So, we don't need to do it at least
for now. Therefore, this and following patches does not assume that there
are no cases that only new lines are collapsible.
Differential Revision: https://phabricator.services.mozilla.com/D124555
`WSRunScanner` is designed for scanning editable nodes, but
`IsVisibleTextNode()` should not check editable state. Therefore, it should
check it without `WSRunScanner` like `IsVisibleBRElement()`.
Differential Revision: https://phabricator.services.mozilla.com/D124554
The method may be used for checking a `<br>` element visibility in non-editable
content too. Therefore, using `WSRunScanner` which is designed for scanning
editable content hits assertions.
The visibility of `<br>` element is considered only with the following
visible thing. If it's followed only by another or current block boundary,
the `<br>` element is invisible. But otherwise, it's visible. So, it needs
to scan following contents to look for first visible thing until reaching
a block boundary. And it does not need to check its previous content.
Differential Revision: https://phabricator.services.mozilla.com/D123876
It's should not cross preformatted line break, therefore, the better name
for it is, it means whether the preformatted white-space is found or not.
Differential Revision: https://phabricator.services.mozilla.com/D123875
Masayuki Nakano
Emilio Cobos Álvarez
Chris Peterson
Ting-Yu Lin