Skip to content

Comments

mail.yahoo.com: caret jumps to the start of the editor after outdenting out of a blockquote#35764

Closed
whsieh wants to merge 1 commit intoWebKit:mainfrom
whsieh:eng/281795
Closed

mail.yahoo.com: caret jumps to the start of the editor after outdenting out of a blockquote#35764
whsieh wants to merge 1 commit intoWebKit:mainfrom
whsieh:eng/281795

Conversation

@whsieh
Copy link
Member

@whsieh whsieh commented Oct 27, 2024

055682d

mail.yahoo.com: caret jumps to the start of the editor after outdenting out of a blockquote
https://bugs.webkit.org/show_bug.cgi?id=282142
rdar://138081308

Reviewed by NOBODY (OOPS!).

In Yahoo Mail, when outdenting to break out of a blockquote element on a line with no content, the
selection is always reset to the top of the editable root. Suppose we're outdenting in a simple DOM
tree on Yahoo:

```
--- Safari, before outdenting

<DIV contenteditable> # Yahoo's root editable element
    <BLOCKQUOTE>
        <DIV>
            Some text
        <DIV>
            # Selection is here.
            <BR>
```

1.  When clicking on the outdent menu item in Yahoo Mail's UI, Yahoo triggers the `Outdent` editing
    command. This executes as expected, and places the ending selection in the correct location
    (depicted below).

```
--- Safari, after outdenting

<DIV contenteditable> # Yahoo's root editable element
    <BLOCKQUOTE>
        <DIV>
            Some text
    # Selection is here
    <BR>
```

2.  However, immediately after this happens, the menu dismisses, which runs some other JavaScript
    that then attempts to "fix up" the DOM selection. This JavaScript always moves the selection to
    the start of the editable root, under 3 specific conditions:

    a. The selection is collapsed.
    b. The editable root (i.e. contenteditable `DIV` marked above) has ≥1 children.
    c. The selection anchor (`getSelection().anchorNode`) is equal to the editable root.

Because we satisfy all three of the above conditions (since the selection anchor is at offset 1 in
the editable root), the selection is always moved to the start, which causes the bug.

This does not happen in Chrome because, upon initially creating the `BLOCKQUOTE` element, Chrome
wraps the block quote in a redundant `DIV`, such that the final selection after outdenting places
the selection anchor under this new `DIV` rather than under the root editable.

```
--- Chrome, after outdenting

<DIV contenteditable> # Editable root
    <DIV>
        <BLOCKQUOTE>
            <DIV>
                Some text
        # Selection is here
        <BR>
```

This also does not happen in Firefox because outdenting creates a DOM structure that has an extra
`DIV` element containing the `BR`:

```
--- Firefox, after outdenting

<DIV contenteditable> # Editable root
    <BLOCKQUOTE>
        <DIV>
            Some text
    <DIV>
        # Selection is here
        <BR>
```

To address this issue, this patch aligns our behavior to Firefox, by ensuring that the placeholder
`BR` above is additionally contained in the document's default paragraph element (i.e. `DIV`). This
approach is a bit less intrusive of a change than adjusting to match Chrome's behavior, but still
avoids this bug.

* LayoutTests/editing/execCommand/outdent-break-with-style-expected.txt:
* LayoutTests/editing/execCommand/outdent-regular-blockquote-expected.txt:
* LayoutTests/editing/execCommand/outdent-selection-expected.txt:
* LayoutTests/editing/execCommand/unindent-nested-blockquote-with-inner-div-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/outdent_1-1000-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/outdent_2001-last-expected.txt:

Rebaseline various layout tests to account for the new behavior.

* Source/WebCore/editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::outdentParagraph):

055682d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
loading 🧪 style loading 🛠 ios loading 🛠 mac loading 🛠 wpe loading 🛠 win
loading 🧪 bindings loading 🛠 ios-sim loading 🛠 mac-AS-debug loading 🧪 wpe-wk2 loading 🧪 win-tests
loading 🧪 webkitperl loading 🧪 ios-wk2 loading 🧪 api-mac loading 🧪 api-wpe
loading 🧪 ios-wk2-wpt loading 🧪 mac-wk1 loading 🛠 wpe-cairo
loading 🧪 api-ios loading 🧪 mac-wk2 loading 🛠 gtk
loading 🛠 vision loading 🧪 mac-AS-debug-wk2 loading 🧪 gtk-wk2
loading 🛠 vision-sim loading 🧪 mac-wk2-stress loading 🧪 api-gtk
⏳ 🧪 vision-wk2 loading 🧪 mac-intel-wk2
loading 🛠 tv loading 🛠 mac-safer-cpp
loading 🛠 tv-sim
loading 🛠 watch
loading 🛠 watch-sim

…ng out of a blockquote

https://bugs.webkit.org/show_bug.cgi?id=282142
rdar://138081308

Reviewed by NOBODY (OOPS!).

In Yahoo Mail, when outdenting to break out of a blockquote element on a line with no content, the
selection is always reset to the top of the editable root. Suppose we're outdenting in a simple DOM
tree on Yahoo:

```
--- Safari, before outdenting

<DIV contenteditable> # Yahoo's root editable element
    <BLOCKQUOTE>
        <DIV>
            Some text
        <DIV>
            # Selection is here.
            <BR>
```

1.  When clicking on the outdent menu item in Yahoo Mail's UI, Yahoo triggers the `Outdent` editing
    command. This executes as expected, and places the ending selection in the correct location
    (depicted below).

```
--- Safari, after outdenting

<DIV contenteditable> # Yahoo's root editable element
    <BLOCKQUOTE>
        <DIV>
            Some text
    # Selection is here
    <BR>
```

2.  However, immediately after this happens, the menu dismisses, which runs some other JavaScript
    that then attempts to "fix up" the DOM selection. This JavaScript always moves the selection to
    the start of the editable root, under 3 specific conditions:

    a. The selection is collapsed.
    b. The editable root (i.e. contenteditable `DIV` marked above) has ≥1 children.
    c. The selection anchor (`getSelection().anchorNode`) is equal to the editable root.

Because we satisfy all three of the above conditions (since the selection anchor is at offset 1 in
the editable root), the selection is always moved to the start, which causes the bug.

This does not happen in Chrome because, upon initially creating the `BLOCKQUOTE` element, Chrome
wraps the block quote in a redundant `DIV`, such that the final selection after outdenting places
the selection anchor under this new `DIV` rather than under the root editable.

```
--- Chrome, after outdenting

<DIV contenteditable> # Editable root
    <DIV>
        <BLOCKQUOTE>
            <DIV>
                Some text
        # Selection is here
        <BR>
```

This also does not happen in Firefox because outdenting creates a DOM structure that has an extra
`DIV` element containing the `BR`:

```
--- Firefox, after outdenting

<DIV contenteditable> # Editable root
    <BLOCKQUOTE>
        <DIV>
            Some text
    <DIV>
        # Selection is here
        <BR>
```

To address this issue, this patch aligns our behavior to Firefox, by ensuring that the placeholder
`BR` above is additionally contained in the document's default paragraph element (i.e. `DIV`). This
approach is a bit less intrusive of a change than adjusting to match Chrome's behavior, but still
avoids this bug.

* LayoutTests/editing/execCommand/outdent-break-with-style-expected.txt:
* LayoutTests/editing/execCommand/outdent-regular-blockquote-expected.txt:
* LayoutTests/editing/execCommand/outdent-selection-expected.txt:
* LayoutTests/editing/execCommand/unindent-nested-blockquote-with-inner-div-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/outdent_1-1000-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/editing/run/outdent_2001-last-expected.txt:

Rebaseline various layout tests to account for the new behavior.

* Source/WebCore/editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::outdentParagraph):
@whsieh whsieh requested a review from rniwa as a code owner October 27, 2024 02:47
@whsieh whsieh self-assigned this Oct 27, 2024
@whsieh whsieh added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Oct 27, 2024
@whsieh whsieh closed this Oct 27, 2024
@whsieh whsieh deleted the eng/281795 branch October 27, 2024 02:48
@whsieh
Copy link
Member Author

whsieh commented Oct 27, 2024

This PR branch was opened by accident (doesn't match the bugzilla bug)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

HTML Editing For bugs in HTML editing support (including designMode and contentEditable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants