Summary: | ASSERT(upstreamStart.deprecatedNode()->isDescendantOf(enclosingBlock(upstreamEnd.deprecatedNode()))) in CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary | ||
---|---|---|---|
Product: | WebKit | Reporter: | Renata Hodovan <rhodovan.u-szeged> |
Component: | HTML Editing | Assignee: | Éva Balázsfalvi <evab.u-szeged> |
Status: | RESOLVED DUPLICATE | ||
Severity: | Normal | CC: | bfulgham, buildbot, darin, evab.u-szeged, jiewen_tan, ossy, rniwa, tkent |
Priority: | P2 | Keywords: | BlinkMergeCandidate |
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=151288 | ||
Bug Depends on: | |||
Bug Blocks: | 116980 | ||
Attachments: |
Description
Renata Hodovan
2013-02-20 08:21:14 PST
Created attachment 233598 [details]
Patch
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 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
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 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
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 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
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? Created attachment 233695 [details]
Patch
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 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
Could someone review this patch for me? Thank you! Comment on attachment 233695 [details]
Patch
A review would be really appreciated. Thanks in advance.
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.
(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. Created attachment 241779 [details]
Patch
Ping? Comment on attachment 241779 [details] Patch This assertion was removed by bug151288. Could you confirm if it is dup of it? (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. *** This bug has been marked as a duplicate of bug 151288 *** |