Bug 51639

Summary: chrome.dll!WebCore::ApplyStyleCommand::applyBlockStyle ReadAV@NULL (64db547804532a84be2e53721e499e9e)
Product: WebKit Reporter: Berend-Jan Wever <skylined>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, feherzs, inferno, jaysoffian, rniwa, tony, webkit.review.bot
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
Attachments:
Description Flags
Repro
none
Patch
none
Patch
none
Patch
none
Patch rniwa: review+, rniwa: commit-queue+

Berend-Jan Wever
Reported 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 ...
Attachments
Repro (420 bytes, application/xhtml+xml)
2010-12-27 05:56 PST, Berend-Jan Wever 		no flags
Details
Patch (2.58 KB, patch)
2011-09-23 12:50 PDT, Jay Soffian 		no flags
DetailsFormatted DiffDiff
Patch (2.59 KB, patch)
2011-09-23 12:58 PDT, Jay Soffian 		no flags
DetailsFormatted DiffDiff
Patch (1.40 KB, patch)
2011-09-26 07:40 PDT, Jay Soffian 		no flags
DetailsFormatted DiffDiff
Patch (1.40 KB, patch)
2011-09-26 08:46 PDT, Jay Soffian 		rniwa: review+
rniwa: commit-queue+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jay Soffian
Comment 1 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
Tony Chang
Comment 2 2011-09-23 10:16:57 PDT
It would be nice to add a layout test to cover BJ's repro.
Jay Soffian
Comment 3 2011-09-23 12:50:28 PDT
Created attachment 108511 [details] Patch
Tony Chang
Comment 4 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.
Jay Soffian
Comment 5 2011-09-23 12:58:10 PDT
Created attachment 108513 [details] Patch
WebKit Review Bot
Comment 6 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>
WebKit Review Bot
Comment 7 2011-09-23 16:23:27 PDT
All reviewed patches have been landed. Closing bug.
Fehér Zsolt
Comment 8 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
Jay Soffian
Comment 9 2011-09-26 07:40:24 PDT
Created attachment 108664 [details] Patch
Jay Soffian
Comment 10 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. :-(
Ryosuke Niwa
Comment 11 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.
Jay Soffian
Comment 12 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.
Jay Soffian
Comment 13 2011-09-26 08:46:28 PDT
Created attachment 108671 [details] Patch
Jay Soffian
Comment 14 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.
Ryosuke Niwa
Comment 15 2011-09-26 09:12:47 PDT
Reopen the bug so that commit queue can land the patch.
Ryosuke Niwa
Comment 16 2011-09-26 09:59:37 PDT
Committed r95958: <http://trac.webkit.org/changeset/95958>
Note You need to log in before you can comment on or make changes to this bug.