Bug 94740 – Crash in EditingStyle::mergeStyle

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

Reduction from http://code.google.com/p/chromium/issues/detail?id=130458
<div>a
    <progress>
        <a style></a>
    </progress>
</div>
<script>
setTimeout(function(){
    document.body.contentEditable = true;
    document.execCommand("selectall")
    document.execCommand("createlink", false, "a")
})
</script>

[Sukolsak Sakshuwong]

Created attachment 160619 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 160619 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        2. The first remove phrase (removeInlineStyle()) thus did not remove <a>.
> +        3. Then, we called fixRangeAndApplyInlineStyle(). In this method, we set
> +           pastEndNode to the next sibling of <progress>.

I don't really follow these two steps. Revise?

> Source/WebCore/ChangeLog:16
> +        4. Then, we called removeStyleFromRunBeforeApplyingStyle(). This method tried

"Then" is redundant given you've numbered them 1-4.

> Source/WebCore/editing/ApplyStyleCommand.cpp:801
> -        next = node->traverseNextNode();
> +        next = editingIgnoresContent(node.get()) ? node->traverseNextSibling() : node->traverseNextNode();

We need to make sure the ignored content doesn't contain pastEndNode.
We should either assert that and/or break the loop when that's not the case.

> Source/WebCore/editing/ApplyStyleCommand.cpp:1051
> +        RefPtr<Node> next = editingIgnoresContent(node) ? node->traverseNextSibling() : node->traverseNextNode();

Ditto.

[Sukolsak Sakshuwong]

Created attachment 160677 [details]
Patch

[Build Bot]

Comment on attachment 160677 [details]
Patch

Attachment 160677 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13621040

[Sukolsak Sakshuwong]

Created attachment 160685 [details]
Patch

[WebKit Review Bot]

Comment on attachment 160685 [details]
Patch

Clearing flags on attachment: 160685

Committed r126865: <http://trac.webkit.org/changeset/126865>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Roger Fong]

The test fails on Apple Windows port.
Any idea why?
Thanks,

Here's the diff:
--- /home/buildbot/slave/win-release-tests/build/layout-test-results/editing/style/apply-style-atomic-expected.txt	2012-08-29 19:43:14.684944200 -0700
+++ /home/buildbot/slave/win-release-tests/build/layout-test-results/editing/style/apply-style-atomic-actual.txt	2012-08-29 19:43:14.683944100 -0700
@@ -3,15 +3,4 @@
 |   href="a"
 |   "<#selection-anchor>1"
 |   <progress>
-|     <a>
-|       style=""
-|       "2"
-|     <shadow:root>
-|       <div>
-|         shadow:pseudoId="-webkit-progress-inner-element"
-|         <div>
-|           shadow:pseudoId="-webkit-progress-bar"
-|           <div>
-|             style="width: -100%;"
-|             shadow:pseudoId="-webkit-progress-value"
-|   <#selection-focus>
+|     "2<#selection-focus>"