Summary: | REGRESSION(r124739): fast/lists/list-marker-remove-crash.html hits an assertion in MoveParagraphs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||
Component: | HTML Editing | Assignee: | Andrei Bucur <abucur> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Critical | CC: | abucur, dstockwell, enrica, gyuyoung.kim, inferno, mifenton, ossy, rakuco, rniwa, simon.fraser, WebkitBugTracker, webkit.review.bot, zan | ||||
Priority: | P1 | Keywords: | AdobeTracked, Gtk, InRadar, LayoutTestFailure | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 79668 | ||||||
Attachments: |
|
Description
Csaba Osztrogonác
2012-08-06 03:23:55 PDT
The patch was rolled out - https://bugs.webkit.org/show_bug.cgi?id=93241 *** This bug has been marked as a duplicate of bug 93241 *** The security fix has no relation to this editing bug. http://trac.webkit.org/changeset/124739 just tries to prevent an empty anonymous block continuation from cleaning up early [and cleanup code didn't exist before https://trac.webkit.org/changeset/120477] This bug will track the editing bug. security bug went in http://trac.webkit.org/changeset/124783 I fixed the expectations, because assertions are valid in debug mode only - https://trac.webkit.org/changeset/124890 I cannot reproduce this assertion failure on my MacPro with Snow Leopard at r125081. (In reply to comment #5) > I cannot reproduce this assertion failure on my MacPro with Snow Leopard at r125081. Did you try to reproduce it in debug mode? :) The assertion is still valid, check the Lion, GTK and Qt _debug_ bots: - http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20%28Tests%29/r125146%20%281718%29/fast/lists/list-marker-remove-crash-stderr.txt - http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r125143%20%2835465%29/fast/lists/list-marker-remove-crash-stderr.txt - http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r125143%20%2824391%29/fast/lists/list-marker-remove-crash-crash-log.txt (In reply to comment #6) > (In reply to comment #5) > > I cannot reproduce this assertion failure on my MacPro with Snow Leopard at r125081. > > Did you try to reproduce it in debug mode? :) Yes! I'm gonna try Lion now :) Yeah, I can reproduce the assertion failure on Lion. Unfortunately, this is one of those impenetrable InsertListCommand bugs. Crashes for me every time I run tests. The bug is still valid. It seems we don't update the end position after updating start in InsertListCommand::listifyParagraph. I'm really not familiar with that code, but common sense dictates we should do that (if start changed, why shouldn't end change as well?). Something around these lines: // We inserted the list at the start of the content we're about to move // Update the start of content, so we don't try to move the list into itself. bug 19066 // Layout is necessary since start's node's inline renderers may have been destroyed by the insertion if (insertionPos == start.deepEquivalent()) { listElement->document()->updateLayoutIgnorePendingStylesheets(); start = startOfParagraph(originalStart, CanSkipOverEditingBoundary); + end = endOfParagraph(start, CanSkipOverEditingBoundary); } After adding that new line the test doesn't crash any more. Is this the right fix? Created attachment 190498 [details]
Patch
Comment on attachment 190498 [details] Patch Clearing flags on attachment: 190498 Committed r144213: <http://trac.webkit.org/changeset/144213> All reviewed patches have been landed. Closing bug. |