Span element gets produced using backspace/delete to merge header with paragraph

• Component: HTML Editing
• Status: RESOLVED FIXED

[Johan "Spocke" Sörlin]

Created attachment 107944 [details]
Test file that shows the bug

A span element with the runtime style of the paragraph gets produced if you backspace/delete to merge a h1 and p element.

When pressing delete at this caret location:
<h1>a|</h1>
<p>b</p>

WebKit produces if P has a color red defined in it's CSS:
<h1>a<span style="color:red">b</span></h1>

Should produce like other browsers:
<h1>ab</h1>

[Ryosuke Niwa]

Created attachment 108096 [details]
Patch

[Ryosuke Niwa]

Thanks for the view, Darin.

cc more people since this is a follow up to the bug 45784.

[Ryosuke Niwa]

Committed r95645: <http://trac.webkit.org/changeset/95645>

[Balazs Kelemen]

The new test fails on Qt-WK2: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r95646%20(12117)/editing/deleting/merge-paragraph-from-p-with-style-pretty-diff.html.
Could you determine whether it is a bad result or it just need to be rebaselined?

[Ryosuke Niwa]

(In reply to comment #4)
> The new test fails on Qt-WK2: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r95646%20(12117)/editing/deleting/merge-paragraph-from-p-with-style-pretty-diff.html.
> Could you determine whether it is a bad result or it just need to be rebaselined?

It seems like Qt-WK2 has a bug in editing delegates. As far as I can tell, only reasonable failure for this test is to have a difference between Mac and non-Mac. Since the said test appears to be passing on Chromium Linux, Qt WebKit1, GTK, etc..., it's very likely that there's some bug in Qt's WK2 code.

[Johan "Spocke" Sörlin]

Created attachment 108356 [details]
Another issue where it produced a span with the inherited runtime style on backspace/delete

It still creates a span with the inherited runtime style from the body or any other element that might be in the chain. It shouldn't produce a span at all like other browsers.

[Ryosuke Niwa]

(In reply to comment #6)
> Created an attachment (id=108356) [details]
> Another issue where it produced a span with the inherited runtime style on backspace/delete
> 
> It still creates a span with the inherited runtime style from the body or any other element that might be in the chain. It shouldn't produce a span at all like other browsers.

Could you give me the list of all test cases so that I can get the idea what kind of behavior you're expecting to get?  Since WebKit preserves inline styles and styles inherited from style rules when copy/paste unlike any other browser, I need to understand exactly when we want to avoid that.

[Johan "Spocke" Sörlin]

Basically it should never use runtimes styles anywhere since the contents edited should be as semantic as possible. Producing a lot of styles is not something one would want in the editor output. Might be useful for a mail client but not for CMS systems, blogs etc. And none of the other browsers work the way WebKit does here so in order to normalize behavior it should retain the inline elements that are there and not produce a new span based on runtime stuff:

For example backspace on this:
<h1>hello</h1>
<p><b><i><span style="color:red">|world</span></i></b></p>

Would produce this:
<h1>hello<b><i><span style="color:red">|world</span></i></b></h1>

And backspace on this:
<h1>hello</h1>
<p>|world</p>

Would produce this regardless of what the CSS might look like:
<h1>hello|world</h1>

Maybe we could get some input from Aryeh Gregor on this since he has all those tests etc for the rich text editing specification and I would guess that the ForwardDelete and Delete execCommands has some tests there maybe it could be extended to make sure that the same results happen on all browsers.

[Ryosuke Niwa]

Okay, this is getting way messier than I wanted. I'm going to roll out 95645 and 95335 (and 95337) and reopen these bugs. Given your new description, the approach I took doesn't work at all.

[Johan "Spocke" Sörlin]

Makes sense. I mailed Aryeh and CC:ed you on how to move forward on this. I know there are differences between browsers when it comes to deleting ranges between blocks with various formatting so the right approach would be to figure out how it should work and then make sure the reference implementation and tests in the spec reflect these edge cases. But it's better to discuss that outside this bug report.

[Ryosuke Niwa]

(In reply to comment #10)
> Makes sense. I mailed Aryeh and CC:ed you on how to move forward on this. I know there are differences between browsers when it comes to deleting ranges between blocks with various formatting so the right approach would be to figure out how it should work and then make sure the reference implementation and tests in the spec reflect these edge cases. But it's better to discuss that outside this bug report.

I'll note, however, that following spec is not necessarily possible at times due to various reasons. I'll investigate further about this issue.

[Aryeh Gregor]

Here are delete and forwardDelete test-cases from my spec:

http://dvcs.w3.org/hg/editing/raw-file/tip/conformancetest/splitruntest.html?delete
http://dvcs.w3.org/hg/editing/raw-file/tip/conformancetest/splitruntest.html?forwarddelete

Most of the failures in WebKit are queryCommand*(), as discussed on whatwg a few days ago.  There are a number of failures in Chrome 15 dev that are related to this bug, such as:


Fail	[["stylewithcss","false"],["delete",""]] "<pre>foo</pre>[]bar" compare innerHTML	assert_equals: Unexpected innerHTML (after normalizing inline style) expected "<pre>foobar</pre>" but got "<pre>foo<span style=\"font-family:serif; white-space:normal\">bar</span></pre>"

Fail	[["stylewithcss","true"],["delete",""]] "<pre>foo</pre>[]bar" compare innerHTML	assert_equals: Unexpected innerHTML (after normalizing inline style) expected "<pre>foobar</pre>" but got "<pre>foo<span style=\"font-family:serif; white-space:normal\">bar</span></pre>"


Firefox 8.0a2 passes both of those tests.  IE and Opera both fail, but only because "delete" means something totally different, so they give "<pre>foo</pre>ar".  If you go to <http://dvcs.w3.org/hg/editing/raw-file/tip/deletetest.html> and enter the test manually as

  ["<pre>foo</pre>[]bar", "delete"]

and hit Backspace when prompted, then it shows their behavior when the user actually hits backspace is the same as Firefox.  (Tested in IE9 and Opera 11.50.  IE10PP2 seems to have a bug here, but still doesn't behave like WebKit.)


The spec's requirements for delete are currently very hard to understand (http://www.w3.org/Bugs/Public/show_bug.cgi?id=13973).  However, the behavior required here is basically

1) After deleting everything that should be deleted, but before merging the start and end blocks, make a list of all the nodes to be moved from the end block to the start block.  (Like "bar" above.)

2) Record the values of that list: <http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#record-the-values>.  This does *not* record the computed/resolved value.  It records the "specified" value for each command, which means any inline styles, plus (for instance) <b> counts as font-weight: bold, <i> counts as font-style: italic, etc.

3) Move all the nodes around.

4) Restore the values that were recorded earlier: <http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#restore-the-values>.  This pushes down and/or forces the value of the node, so that it has the same value as before.