Code like the one from comment 7 seems reasonable. Let's try to disable
prefixed transitions along with prefixed transforms in order to minimize
the compat fallout from CSS zoom.
While I was at it I also fixed the prefixed animation entries in the
property database and crashtests. But those aliases remain enabled.
Differential Revision: https://phabricator.services.mozilla.com/D192129
This is a simple mistake of the previous patch. The code path is originally for
a left click handling. Therefore, there is no check whether the click point is
in a selection range because in that case, left click should collapse selection.
However, the selection should not be collapse when it's a right click in a
selection range because the context menu target is the selection range.
The `ui.mouse.right_click.collapse_selection.stop_if_non_collapsed_selection`
pref is set to `true` by default. Therefore, the existence of a non-collapsed
selection has hidden this bug. However, if it's set to `false`, this always
happens. Additionally, editor does not set ancestor limiter of selection if the
root element is editable [1]. I think that we can now remove this special path,
but it's risky. Therefore, this patch just adds the new check into the method.
1. https://searchfox.org/mozilla-central/rev/ce4599d3d4e20e34cb6db68f255c3dff1aec246b/editor/libeditor/EditorBase.cpp#5448,5450-5451
Differential Revision: https://phabricator.services.mozilla.com/D193529
They check ancestor limiter when they are climbing up the DOM tree if and only
if the ancestor does not have a sibling. However, this causes moving into
a sibling of the ancestor limiter.
Differential Revision: https://phabricator.services.mozilla.com/D192528
Gecko starts referring computed style to consider whether block or inline and
`HTMLEditUtils::IsEmptyNode` is used with them to check a node like these:
https://searchfox.org/mozilla-central/rev/40d51bef58b8e901d6ab4b60dd280f372a0e417d/editor/libeditor/HTMLEditUtils.h#543,554
However, `IsEmptyNode` still checks only the tag names when the caller wants
to treat "list items" and/or "table cells" are visible (not empty). Therefore,
this mismatch may cause the assertion failure reported in the bug.
From code design point of view, `HTMLEditUtils::IsListItem` and
`HTMLEditUtils::IsTableCell` should refer the computed style. However, the
former is mostly used for checking the logical structure of HTML, and in the
latter case, we should keep the table structure even if table cells are styled
as inline because browsers do not support table structure recreation except
the Gecko specific editing UI (called inline table editor). Therefore, it does
not make sense that we would change them.
Instead, we should change `IsEmptyNode` which is used only for visibility check.
So, using computed style is always reasonable.
Note that this patch changes the behavior in some edge cases. The method always
treat a list item or a table cell which contains a sub-list or a sub table.
However, I would not like to add new `EmptyCheckOption` for it to check the
complicated style check. Therefore, this patch changes the behavior to treat
them visible if and only if they have another list item or another table cell.
Basically, `<table>` and list elements should have a least one table cell or
one list item. Therefore, this change should not appear in the web apps in the
wild.
Differential Revision: https://phabricator.services.mozilla.com/D192442
It moves nodes in a range to new place one by one. At this time, the moved
position range is not tracked. Therefore, if the DOM tree is unexpectedly
changed by `HTMLEditor` itself, the range gets broken. E.g., in this case,
deleting empty inline element causes the range after the source position is
broken.
Differential Revision: https://phabricator.services.mozilla.com/D192180
Now, all callers of them do not require to check `<dl>`, `<dt>` and `<dd>` check
if it's in the HTML format block mode. Therefore, we can move the check into
`HTMLEditor::IsFormatElement` to make the caller side simpler.
Additionally, this patch replaces some places with
`HTMLEditUtils::IsFormatElementForParagraphState` because the paths handle only
XUL paragraph state command with assertion check.
Depends on D190902
Differential Revision: https://phabricator.services.mozilla.com/D191604
Currently, we extend selection to outside block boundaries if selection starts
and/or ends very start/end of block boundaries. Then, replace most-distant
ancestor format node, but this is odd because the innermost format is applied
to the text and the other browsers targets the innermost ancestor formant node.
Therefore, we should align to them.
Additionally, our replacing code for `<dd>` and `<dt>` has not been implemented
yet. I guess that the old `HTMLEditUtils::IsFormatNode` returned `false` for
`<dd>`, `<dl>` and `<dt>` and it requires additional work for splitting parent
`<dl>` too. Now, we can do it with simple code.
Finally, we created new format block element if `<br>` element appears, but
the other browsers moves it into the new format node simply. Therefore, we
should follow them (about `<blockquote>`, we've already done this within
the different path).
Differential Revision: https://phabricator.services.mozilla.com/D190902
`Document.execCommand` accepts `dl`, `dt` and `dd` as a format block. However,
`ParagraphStateAtSelection` ignores them as not a format block element.
Therefore, it causes odd result of `queryCommandValue` after applying the
format. E.g., `execCommand("dd")` to `<p>[foobar]</p>` correctly updates the
HTML to `<dl><dd>foobar</dd></dl>`, however, `queryCommandValue("dd")` returns
empty string rather than `dd`.
Note that Chromium returns inclusive ancestor format element of common ancestor
of the range. However, the behavior is odd because it makes the indeterm
state never `true` and the result is not used as the target of format block
command. So, this patch does not follow them at least for now.
On the other hand, this patch adds special case for `dl`. With user's
operation in valid HTML structure, selection range never selects `<dl>`
children directly. However, may cross `<dt>` and `<dd>` elements.
Then, let's return `dl` for compatibility with Chromium because `<dl>`
ancestor may be the format block target.
Differential Revision: https://phabricator.services.mozilla.com/D190901
I realized that our `HTMLEditUtils::IsFormatNode` is not maintained different
from the other browsers. Therefore, only we do not check new elements defined
after HTML 4.01. This patch aligns the list of the format elements to the
others [1].
Then, this also changes some expectations of `editing/run/formatblock.html`
to align common behavior of the browsers.
Note that we mapped `formatBlock` of `execCommand` to `cmd_paragraphState`,
and the XUL command handles `<blockquote>` in a different path [2] and the
behavior is pretty different from the other formatBlock command implementations.
Therefore, this patch creates new command for `formatBlock` and makes
`HTMLEditor` switch behavior in any places.
1. ba50f40fc4:third_party/WebKit/WebCore/editing/FormatBlockCommand.cpp;l=114-134
2. https://searchfox.org/mozilla-central/rev/6602bdf9fff5020fbc8e248c963ddddf09a77b1b/editor/libeditor/HTMLEditor.cpp#2461-2474
Differential Revision: https://phabricator.services.mozilla.com/D190900
The handler has an early-return block for the case inserting empty text.
However, it's done before deleting selection, but it should be done after that.
Differential Revision: https://phabricator.services.mozilla.com/D190904
This makes number localization cheaper / halves the time in the
microbenchmark.
Change the content-language handling to use atoms. This exposes some
interesting inconsistencies but I tried not to change behavior there.
Differential Revision: https://phabricator.services.mozilla.com/D191174
As the comment in the method, `CurrentBlockBoundary` may be set when `mContent`
is not a block. However, it's not allowed that there is an editable block
ancestor of `mContent` in same editing host (in this case, `mContent` should be
the block). Therefore, it should compute ancestor block element and check
whether it's not in same editing host if `mContent` is editable and connected.
Differential Revision: https://phabricator.services.mozilla.com/D190644
Sorry this is not a particularly easy patch to review. But it should be
mostly straight-forward.
I kept Document::Dispatch mostly for convenience, but could be
cleaned-up too / changed by SchedulerGroup::Dispatch. Similarly maybe
that can just be NS_DispatchToMainThread if we add an NS_IsMainThread
check there or something (to preserve shutdown semantics).
Differential Revision: https://phabricator.services.mozilla.com/D190450
This patch is related to Bug 1840822, which fixes some issues
regarding additional invisible linebreaks needed at the end of
contenteditables.
This patch in particular fixes the case where the user
presses enter at the end of a pre-formatted
contenteditable, e.g.:
```
<span contenteditable style="display:block;white-space:pre-line">[]</span>
```
Prior to this patch, only one `<br>` was added,
the invisible padding linebreak was missing.
With this patch applied, the behaviour should be as expected:
```
<span contenteditable style="display:block;white-space:pre-line">\n<br></span>
```
Differential Revision: https://phabricator.services.mozilla.com/D190217
The other browsers move focus and `Selection` whe right click even if the
clicked element is not editable and even if there is a non-collapsed selection.
Fortunately, we already have similar code for the middle button press.
Therefore, we can make it run when the pressed button is the secondary button.
This also fixes bug 416546 and does not resurrect bug 709476.
However, this patch adds 2 prefs for making users customizable. Our traditional
behavior is, we never collapse non-collapses selection with a right click even
if clicked outside the selection. This allows users to open context menu for
selected text much easier. Therefore, even though the behavior is different
from the others, we should keep the traditional behavior, but some users may
want the other browsers' behavior instead. For them, this should be switchable
by a pref.
Additionally, I'm still not sure collapsing selection with a right click in
non-editable content especially for users using the caret browsing mode.
Therefore, for making things safer, this adds a pref to disable the new behavior
in the non-editable content.
Differential Revision: https://phabricator.services.mozilla.com/D189991
If `HTMLEditor::HandleInsertBRElement` inserts an invisible `<br>` element
after a `<br>` element which is requested, new caret position should be
at the invisible `<br>` element.
Differential Revision: https://phabricator.services.mozilla.com/D189997
Without this patch, the following test newly fails:
```
/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText
FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: div) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"
FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: p) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"
```
The reasons is, the inlined editing host is at end of the `<body>`, therefore,
even though the editing host itself is `inline`, it needs a padding `<br>` to
make it the new line visible.
However, `HTMLEditUtils::GetElementOfImmediateBlockBoundary` does not return
`<body>` because the editing host is followed by `<script>` which has
a text node which is not white-spaces only. Therefore,
`HTMLEditUtils::IsVisibleBRElement` considers the `<br>` element at end of the
editing host is "visible".
Differential Revision: https://phabricator.services.mozilla.com/D188599
Blink [1] and WebKit [2] refers `display-outside` value when they consider whether an
element is a block or an inline.
However, our editor refers HTML default style instead. Therefore, our editor
cannot handle the following cases:
* If a container block is a non-HTML element whose `display` style is `block`.
* If a container `<div>` etc is styled as `inline` and typing `Enter` in it.
* If a container `<span>` is styled as `block` and it ends with spaces.
For making users get better result, we should follow the other browsers.
However, this is too risky change. Therefore, this patch enables new behavior
only in the Nightly channel and early beta builds to collect feedback from
testers.
The big rules of checking block vs. inline are:
* When we handle block level edit actions such as formatting block, indenting
or outdenting selection, making or removing list, we should keep referring
the HTML default style because the other browsers do so and the commands are
intended for modifying the HTML structure.
* Otherwise, we should refer the computed style of the block. However, if
working with non-connected elements, we may need a special handling that is
falling back to refer the HTML default style because `HTMLEditorDataTransfer`
work with non-connected document fragments.
* Finally, if we check visibility of collapsible white-spaces and `<br>`s, we
should refer computed style. However, in this case, we may need to treat
ancestor `inline-block`s as block too, but for siblings, we should refer only
`display-outside`.
1. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/editing/editing_utilities.cc;l=779;drc=31fb07c05718d671d96c227855bfe97af9e3fb20
2. https://searchfox.org/wubkat/rev/b054636fffeffa0314914a11ce39725a3057131e/Source/WebCore/editing/Editing.cpp#317
Differential Revision: https://phabricator.services.mozilla.com/D188598
Cristian Tuns
Emilio Cobos Álvarez
Masayuki Nakano
Divyanshu Agrawal
Ajay
Mark Banner
Edgar Chen
Sylvestre Ledru
Cristina Horotan
Tom Marble
Natalia Csoregi
Noemi Erli
Jan-Niklas Jaeschke
serge-sans-paille
Jan Varga
Joel Maher
Olli Pettay
pommicket
Cosmin Sabou
Gregory Pappas