RESOLVED DUPLICATE of bug 151288110350
ASSERT(upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstreamEnd.deprecatedNode()))) in CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary 
https://bugs.webkit.org/show_bug.cgi?id=110350
Summary ASSERT(upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstream...
Renata Hodovan
Reported 2013-02-20 08:21:14 PST
Running the test below in Debug we got an assertion failure: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff42a85bc in WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary ( this=0x848100, pos=...) at /home/reni/Data/REPOS/webkit/Source/WebCore/editing/CompositeEditCommand.cpp:947 947 ASSERT(upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstreamEnd.deprecatedNode()))); The test: <html> <body> <ul style="height: 20px;"></ul> <div> <div></div> <a target="foo">more</a> </div> <script> document.designMode = "on"; document.execCommand("SelectAll"); document.execCommand("CreateLink", 0, 'foo'); document.execCommand("SelectAll"); document.execCommand("JustifyCenter"); </script> </body> </html>
Attachments
Patch (4.27 KB, patch)
2014-06-23 05:11 PDT, Éva Balázsfalvi 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (503.80 KB, application/zip)
2014-06-23 06:39 PDT, Build Bot 		no flags
Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (505.02 KB, application/zip)
2014-06-23 07:35 PDT, Build Bot 		no flags
Details
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (532.78 KB, application/zip)
2014-06-23 10:15 PDT, Build Bot 		no flags
Details
Patch (4.26 KB, patch)
2014-06-24 05:10 PDT, Éva Balázsfalvi 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (542.83 KB, application/zip)
2014-06-24 08:22 PDT, Build Bot 		no flags
Details
Patch (4.57 KB, patch)
2014-11-18 04:08 PST, Éva Balázsfalvi 		ossy: review-
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Éva Balázsfalvi
Comment 1 2014-06-23 05:11:51 PDT
Created attachment 233598 [details] Patch
Build Bot
Comment 2 2014-06-23 06:39:36 PDT
Comment on attachment 233598 [details] Patch Attachment 233598 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4992311616339968 New failing tests: editing/execCommand/justify-center-empty-list-crash.html
Build Bot
Comment 3 2014-06-23 06:39:38 PDT
Created attachment 233601 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 4 2014-06-23 07:35:30 PDT
Comment on attachment 233598 [details] Patch Attachment 233598 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6261390700445696 New failing tests: editing/execCommand/justify-center-empty-list-crash.html
Build Bot
Comment 5 2014-06-23 07:35:33 PDT
Created attachment 233605 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 6 2014-06-23 10:15:42 PDT
Comment on attachment 233598 [details] Patch Attachment 233598 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4728393929064448 New failing tests: editing/execCommand/justify-center-empty-list-crash.html media/W3C/video/networkState/networkState_during_loadstart.html
Build Bot
Comment 7 2014-06-23 10:15:45 PDT
Created attachment 233615 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Csaba Osztrogonác
Comment 8 2014-06-24 03:05:14 PDT
Comment on attachment 233598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233598&action=review > LayoutTests/editing/execCommand/justify-center-empty-list-crash.html:12 > + document.body.textContent = 'PASS if Blink doesn\'t crash.'; Blink?
Éva Balázsfalvi
Comment 9 2014-06-24 05:10:07 PDT
Created attachment 233695 [details] Patch
Build Bot
Comment 10 2014-06-24 08:22:15 PDT
Comment on attachment 233695 [details] Patch Attachment 233695 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6315280166813696 New failing tests: media/track/add-and-remove-track.html fullscreen/video-cursor-auto-hide.html
Build Bot
Comment 11 2014-06-24 08:22:17 PDT
Created attachment 233706 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Éva Balázsfalvi
Comment 12 2014-07-23 04:24:06 PDT
Could someone review this patch for me? Thank you!
Éva Balázsfalvi
Comment 13 2014-08-29 02:33:00 PDT
Comment on attachment 233695 [details] Patch A review would be really appreciated. Thanks in advance.
Darin Adler
Comment 14 2014-08-29 09:17:37 PDT
Comment on attachment 233695 [details] Patch This patch seems kind of strange. It removes an assertion, without any explanation of why except that Chromium already removed it. We need to explain changes. It’s obvious that removing an assertion will make the crash intentionally caused by the assertion go away. What’s not obvious is whether there is some kind of problem left behind. Some editing export should review this and explain why no assertion is needed here.
Csaba Osztrogonác
Comment 15 2014-11-18 02:14:17 PST
(In reply to comment #14) > Comment on attachment 233695 [details] > Patch > > This patch seems kind of strange. It removes an assertion, without any > explanation of why except that Chromium already removed it. We need to > explain changes. It’s obvious that removing an assertion will make the crash > intentionally caused by the assertion go away. What’s not obvious is whether > there is some kind of problem left behind. Some editing export should review > this and explain why no assertion is needed here. https://src.chromium.org/viewvc/blink?revision=175118&view=revision explained that this assert was based on false assumption and added a test case where this assert fires incorrectly.
Éva Balázsfalvi
Comment 16 2014-11-18 04:08:53 PST
Created attachment 241779 [details] Patch
Éva Balázsfalvi
Comment 17 2014-11-21 05:33:11 PST
Ping?
Csaba Osztrogonác
Comment 18 2016-03-10 07:42:39 PST
Comment on attachment 241779 [details] Patch This assertion was removed by bug151288. Could you confirm if it is dup of it?
Brent Fulgham
Comment 19 2016-08-03 11:04:23 PDT
(In reply to comment #18) > Comment on attachment 241779 [details] > Patch > > This assertion was removed by bug151288. Could you confirm if it is dup of > it? This is a duplicate. The test case from the reporter does not assert any longer.
Brent Fulgham
Comment 20 2016-08-03 11:04:38 PDT
*** This bug has been marked as a duplicate of bug 151288 ***
Note You need to log in before you can comment on or make changes to this bug.