Bug 216739

Summary: Selection API: Further improvements to VisibleSelection, FrameSelection, and DOMSelection to preserve anchor and focus
Product: WebKit Reporter: Darin Adler <darin>
Component: HTML EditingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, ews-watchlist, mifenton, rniwa, sam, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=216748
Bug Depends on: 216778    
Bug Blocks: 216325, 218276    
Attachments:
Description Flags
Patch
rniwa: review+, ews-feeder: commit-queue-
Patch for landing none

Description Darin Adler 2020-09-19 18:47:50 PDT
Selection API: Further improvements to VisibleSelection, FrameSelection, and DOMSelection to preserve anchor and focus
Comment 1 Darin Adler 2020-09-19 19:47:47 PDT
Created attachment 409222 [details]
Patch
Comment 2 Ryosuke Niwa 2020-09-19 21:40:27 PDT
Comment on attachment 409222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409222&action=review

Looks like editing/execCommand/insert-list-nested-with-orphaned.html is failing legitimately.

> Source/WebCore/editing/VisibleSelection.cpp:234
> +    m_baseIsFirst = m_base <= m_extent;

We may need to take care of the case when m_base / m_extent turns until null even though m_anchor / m_focus aren't null.
Comment 3 Darin Adler 2020-09-20 11:49:11 PDT
(In reply to Ryosuke Niwa from comment #2)
> Looks like editing/execCommand/insert-list-nested-with-orphaned.html is
> failing legitimately.

I’ve been studying the test, and I think the change in behavior is a small progression. The test does InsertOrderedList on a selection, and the new behavior is that after insertion the whole thing is still selected. The old behavior was that the caret was at the beginning of the line of the last list item. I don’t see this behavior, changing a selection into a caret, as intentional in the implementation and it’s not done in any normal case.

What I don’t understand yet is why the selection anchor and focus are not visible in the dump. Trying to figure that out now.
Comment 4 Darin Adler 2020-09-20 11:53:26 PDT
(In reply to Darin Adler from comment #3)
> What I don’t understand yet is why the selection anchor and focus are not
> visible in the dump. Trying to figure that out now.

Looks like dump-as-markup.js only dumps an anchor, focus, or caret indicator when they happen to have a text node as the container node, and misses them when the node is a non-text node. This seems like something we should fix by enhancing dump-as-markup.js, but further explains what is happening here.

The caret is not going to move itself into a text node any more, even if it is a caret! It was doing that before as non-standard behavior. We set the selection to the children of a node, so that node is going to be the container specified in the selection endpoints, no reason that they would move themselves to be in a text node.
Comment 5 Darin Adler 2020-09-20 12:00:55 PDT
Created attachment 409240 [details]
Patch for landing
Comment 6 Darin Adler 2020-09-20 12:37:50 PDT
Committed r267329: <https://trac.webkit.org/changeset/267329>
Comment 7 Radar WebKit Bug Importer 2020-09-21 12:02:45 PDT
<rdar://problem/69318406>