Bug 51639

Summary:

chrome.dll!WebCore::ApplyStyleCommand::applyBlockStyle ReadAV@NULL (64db547804532a84be2e53721e499e9e)

Product:

WebKit

Reporter:

Berend-Jan Wever <skylined>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, feherzs, inferno, jaysoffian, rniwa, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Windows Vista

Attachments:

Description	Flags
Repro	none
Patch	none
Patch	none
Patch	none
Patch	rniwa: review+, rniwa: commit-queue+

Description Berend-Jan Wever 2010-12-27 05:56:11 PST

Created attachment 77494 [details]
Repro

http://code.google.com/p/chromium/issues/detail?id=68085

Repro:
<html xmlns="http://www.w3.org/1999/xhtml">
  <head>
    <style>
      *{
        text-decoration:blink line-through;
      }
    </style>
    <script>
      function go() {
        document.execCommand("SelectAll");
        document.execCommand("JustifyRight");
        document.execCommand("JustifyNone");
      }
    </script>
  </head>
  <body onload="go()" contenteditable="true">
    <svg>x</svg>
  </body>
</html>

id:             chrome.dll!WebCore::ApplyStyleCommand::applyBlockStyle ReadAV@NULL (64db547804532a84be2e53721e499e9e)
description:    Attempt to read from unallocated NULL pointer+0x24 in chrome.dll!WebCore::ApplyStyleCommand::applyBlockStyle
application:    Chromium 10.0.623.0
stack:          chrome.dll!WebCore::ApplyStyleCommand::applyBlockStyle
                chrome.dll!WebCore::ApplyStyleCommand::doApply
                chrome.dll!WebCore::EditCommand::apply
                chrome.dll!WebCore::applyCommand
                chrome.dll!WebCore::Editor::applyParagraphStyle
                chrome.dll!WebCore::executeApplyParagraphStyle
                chrome.dll!WebCore::executeJustifyLeft
                chrome.dll!WebCore::Editor::Command::execute
                chrome.dll!WebCore::Document::execCommand
                chrome.dll!WebCore::DocumentInternal::execCommandCallback
                chrome.dll!v8::internal::HandleApiCallHelper<...>
                chrome.dll!v8::internal::Builtin_HandleApiCall
                chrome.dll!v8::internal::Invoke
                chrome.dll!v8::internal::Execution::Call
                ...

Comment 1 Jay Soffian 2011-09-22 21:08:41 PDT

I think this is a dupe of/fixed by https://bugs.webkit.org/show_bug.cgi?id=67765

Comment 2 Tony Chang 2011-09-23 10:16:57 PDT

It would be nice to add a layout test to cover BJ's repro.

Comment 3 Jay Soffian 2011-09-23 12:50:28 PDT

Created attachment 108511 [details]
Patch

Comment 4 Tony Chang 2011-09-23 12:54:51 PDT

Comment on attachment 108511 [details]
Patch

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

> w/LayoutTests/ChangeLog:8
> +Add repro for a crash inside WebCore::ApplyStyleCommand::applyBlockStyle. Fixed by r94840.

This should be indented.

Comment 5 Jay Soffian 2011-09-23 12:58:10 PDT

Created attachment 108513 [details]
Patch

Comment 6 WebKit Review Bot 2011-09-23 16:23:22 PDT

Comment on attachment 108513 [details]
Patch

Clearing flags on attachment: 108513

Committed r95885: <http://trac.webkit.org/changeset/95885>

Comment 7 WebKit Review Bot 2011-09-23 16:23:27 PDT

All reviewed patches have been landed.  Closing bug.

Comment 8 Fehér Zsolt 2011-09-26 07:09:24 PDT

This test failed after the patch:
editing/style/justify-without-enclosing-block.xhtml

--- /ramdisk/qt-linux-32-debug/build/layout-test-results/editing/style/justify-without-enclosing-block-expected.txt 
+++ /ramdisk/qt-linux-32-debug/build/layout-test-results/editing/style/justify-without-enclosing-block-actual.txt 
@@ -1,5 +1,22 @@
-execCommand("JustifyNone") was crashing inside WebCore::ApplyStyleCommand::applyBlockStyle.
-See https://bugs.webkit.org/show_bug.cgi?id=51639
-The test has passed if it does not crash.
-
-PASS
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x126
+  RenderBlock {html} at (0,0) size 800x126
+    RenderBody {body} at (8,8) size 784x110
+      RenderBlock {div} at (0,0) size 784x22
+        RenderText {#text} at (0,0) size 696x22
+          text run at (0,0) width 696: "execCommand(\"JustifyNone\") was crashing inside WebCore::ApplyStyleCommand::applyBlockStyle."
+      RenderBlock {div} at (0,22) size 784x22
+        RenderText {#text} at (0,0) size 349x22
+          text run at (0,0) width 349: "See https://bugs.webkit.org/show_bug.cgi?id=51639"
+      RenderBlock {div} at (0,44) size 784x22
+        RenderText {#text} at (0,0) size 261x22
+          text run at (0,0) width 261: "The test has passed if it does not crash."
+      RenderBlock {div} at (0,66) size 784x22
+        RenderBR {br} at (0,0) size 0x22
+      RenderBlock {div} at (0,88) size 784x22
+        RenderText {#text} at (0,0) size 40x22
+          text run at (0,0) width 40: "PASS"
+      RenderBlock (anonymous) at (0,110) size 784x0
+selection start: position 0 of child 0 {#text} of child 0 {div} of body
+selection end:   position 4 of child 0 {#text} of child 4 {div} of body

Comment 9 Jay Soffian 2011-09-26 07:40:24 PDT

Created attachment 108664 [details]
Patch

Comment 10 Jay Soffian 2011-09-26 07:41:57 PDT

(In reply to comment #9)
> Created an attachment (id=108664) [details]
> Patch

I think this should cause the test output to match the expectation. Sorry for the trouble, I'm new at this and sorta expected it wouldn't be possible to land a broken test. :-(

Comment 11 Ryosuke Niwa 2011-09-26 08:37:49 PDT

Comment on attachment 108664 [details]
Patch

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

> w/LayoutTests/editing/style/justify-without-enclosing-block.xhtml:14
> +            window.layoutTestController.dumpAsText();

You don't really need window. on the second line.

Comment 12 Jay Soffian 2011-09-26 08:44:31 PDT

(In reply to comment #11)
> (From update of attachment 108664 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108664&action=review
> 
> > w/LayoutTests/editing/style/justify-without-enclosing-block.xhtml:14
> > +            window.layoutTestController.dumpAsText();
> 
> You don't really need window. on the second line.

Okay, I cut and paste from 4230923.html. Will fix.

Comment 13 Jay Soffian 2011-09-26 08:46:28 PDT

Created attachment 108671 [details]
Patch

Comment 14 Jay Soffian 2011-09-26 09:09:51 PDT

Comment on attachment 108671 [details]
Patch

I don't have a way to test the expectations right now. If the new expectations could be tested before landing, that would be great.

Comment 15 Ryosuke Niwa 2011-09-26 09:12:47 PDT

Reopen the bug so that commit queue can land the patch.

Comment 16 Ryosuke Niwa 2011-09-26 09:59:37 PDT

Committed r95958: <http://trac.webkit.org/changeset/95958>