ASSERTION FAILED: destination.deepEquivalent().anchorNode()->inDocument() third_party/WebKit/Source/WebCore/editing/CompositeEditCommand.cpp(987) : void WebCore::CompositeEditCommand::moveParagraphs(const WebCore::VisiblePosition&, const WebCore::VisiblePosition&, const WebCore::VisiblePosition&, bool, bool) [28191:28191:1053796965558:ERROR:process_util_posix.cc(108)] Received signal 11 It may happen in other places but this is the first bot I see doing this.
Started happening after http://trac.webkit.org/changeset/81165 landed.
This is asserting on Windows, too: http://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r81179%20(26384)/editing/style/iframe-onload-crash-crash-log.txt
Mac editing behavior masks this bug, but in the end it comes down to issues with canHaveChildrenForEditing, which failed to take into account whether the node actually had children. Any node with children should return true for canHaveChildrenForEditing. I'm testing this change before uploading, but I've validated it fixes this crash.
Created attachment 85895 [details] Patch
Comment on attachment 85895 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85895&action=review > Source/WebCore/editing/htmlediting.cpp:92 > + || node->hasChildNodes()); Now that I think about it, it's odd that a function named "canHaveChildrenForEditing" changes its return value depending on whether or not the node has children. Can we add this fix in editingIgnoresContent? I started to think that these two functions are really wrong.
Our note: return !node->hasTagName(hrTag) && // only if no children !node->hasTagName(brTag) && // always !node->hasTagName(imgTag) && // always !node->hasTagName(buttonTag) && // editable? !node->hasTagName(inputTag) && // shadow !node->hasTagName(textareaTag) && // shadow !node->hasTagName(objectTag) && // useFallbackContents returns false !node->hasTagName(iframeTag) && // always !node->hasTagName(embedTag) && // when render isn't RenderEmbeddedObject? !node->hasTagName(appletTag) && // when renderer isn't RenderApplet?? !node->hasTagName(selectTag) && // shadow !node->hasTagName(datagridTag) && #if ENABLE(WML) !node->hasTagName(WMLNames::doTag) && // ?? #endif !node->isTextNode();
Can we get something in to fix this or rollout the original while it is being addressed?
(In reply to comment #7) > Can we get something in to fix this or rollout the original while it is being addressed? We're working on it. We should be able to fix it by tomorrow. r81165 was indeed a correct change it's just that it revealed another editing bug. I'd rather not revert the patch given that the crash doesn't seem to reproduce on release build.
The test is also failing on GTK debug build as expected: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r81213%20(20426)/editing/style/iframe-onload-crash-stderr.txt
I added this test to the Windows Skipped file. Please remove it when fixing this bug.
Also skipped in GTK: http://trac.webkit.org/changeset/81246 Please unskip when landing the fix!
Comment on attachment 85895 [details] Patch r- I think there are comments waiting to be addressed. Plus there are at least three skipped files that should be modified which aren't in this patch. Please consider the cost imposed on lots of people when leaving stuff like this in the tree. (Several people had to investigate this failure, search bugs, modify skipped list, add comments here, etc.) In order words, it may seem good to leave an improvement in the tree but it imposes a cost on lots of other people. In addition leaving asserts in the tree makes it harder for people to use debug builds to test things because they may hit this assert and think something it wrong etc. If an assert will only take a short time to fix, then it will only take a short while to get in the original patch. If it will take along while to fix, then the patch shouldn't be left in anyway. In short, please consider rolling out in the future.
Created attachment 85953 [details] Patch
Committed r81266: <http://trac.webkit.org/changeset/81266>
Yeah!