[Webkit-unassigned] [Bug 112240] execCommand("RemoveFormat") might remove format after the selection

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Claudio Saavedra <csaavedra at igalia.com>  2013-04-11 08:30:52 PST ---
I've spent some time learning my way through the editing code and I've found a couple of things that might be leading to this bug.

In order to explain this let's use a simpler HTML fragment

  <b><div>Foo</div>Bar</b>

for which, selection starts right before "Foo" and ends right before "Bar" (using square brackets, that would look like this):

  <b><div>[Foo</div>]Bar</b>

All of the following happens in WebCore::ApplyStyleCommand::removeInlineStyle(). The first thing I've noticed is that in that method, pushDownInlineStyleAroundNode() is called for both the start and the end node. The reason why this is done is to ensure that we preserve the style to be removed around the selection. However, for the start node, it is checked whether the current position is in the end of the containing node, in which case what is used to push down the style is the next visually distinct candidate:

    // If the pushDownStart is at the end of a text node, then this node is not fully selected.
    // Move it to the next deep quivalent position to avoid removing the style from this node.
    // e.g. if pushDownStart was at Position("hello", 5) in <b>hello<div>world</div></b>, we want Position("world", 0) instead.

In the test case we have, there is an analogous situation but for the end node: pushDownEnd is at the beginning of a text node, therefore this node is not really selected. I think, in this case, it makes sense to use the previous visually distinct candidate to push down the style (I have tested this single change locally and all editing tests pass).

After doing that change, and after running pushDownInlineStyleAroundNode() for both the pushDownStart and pushDownNode nodes, the tree is changed to the following:

  <div>Foo</div><b>Bar</b>

This is already looking good (without these changes, we would instead already have "<div>Foo</div>Bar"). Now comes the next oddity in the code. The block of code that follows takes care of traversing the tree from the start until the end nodes, and removing the format for all nodes in between, as long as they are fully selected. This is ensured by a call to

  nodeFullySelected(node, start, end)

for each HTML element node in the range, including the <B> one that has been pushed down. In particular, for the <B> node, one would expect nodeFullySelected() to return false, as the position where the selection ends is _before_ the last position in the node. Contrarily to this, the function returns true and the <B> node is removed, yielding

  <div>Foo</div>Bar

as the resulting HTML.

I have yet to find why nodeFullySelected() is returning true here, but I wanted to share this before continuing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.