[Webkit-unassigned] [Bug 194143] Unable to move selection into editable roots with 0 height

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=194143

--- Comment #3 from Wenson Hsieh <wenson_hsieh at apple.com> ---
> However, there's a third test failure I have yet to figure out:
> editing/pasteboard/styled-element-markup.html...

So in this test, we try to set the DOMSelection to offset 0 in an editable div, and then we select all, copy, and finally paste. It's important to note that in this test, these steps happen _before_ the image element has loaded, which makes the height of the editable element's renderer 0 because the editable div is otherwise empty.

We're getting different results after the change because we're considering the position Offset(<div>, 0) to be a visible position, whereas we didn't before (instead, the position of Offset(<div>, 0) would be canonicalized to Before(<img>)). Prior to this change, the following would happen when canonicalizing the Position Offset(<div>, 0):

    1. We check upstream and downstream Positions (in this case, they're both still Offset(<div>, 0)) and see if they're candidates by invoking Position::isCandidate.
    2. In Position::isCandidate, the <div>'s RenderBlock's height is 0 because the image hasn't loaded, so we bail and return false.
    3. We continue canonicalization by searching the next and previous positions, and eventually settle on Before(<img>).

After the change, when calling Position::isCandidate in step (2), we notice that the anchor node is a root editable element, despite its RenderBlock's height being 0. This means we'll enter the if statement, observe that hasRenderedNonAnonymousDescendantsWithHeight returns false, and return true from Position::isCandidate because we're at the first editing position in the node.

A few observations:

- It seems like this test was intended to check the case where the image element is not empty (i.e. the image has loaded). And indeed, putting this logic in the load event handler causes the test to pass.
- In the case of a br element instead of an image, we will canonicalize Offset(<div>, 0) to Before(<br>) because the br element has a nonzero height. This means hasRenderedNonAnonymousDescendantsWithHeight returns true, and then we return false from Position::isCandidate for Offset(<div>, 0) because we are not at an editing boundary.
- It's not clear to me whether the new behavior is wrong in this case. One way to ensure that the old behavior remains is to not only check `m_anchorNode->isRootEditableElement()`, but additionally require the Position to be either at an editing boundary, or be the only editing position in the editable root.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190203/ca4a0cdb/attachment-0001.html>