122097 – ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setTextDecorationProperty

RESOLVED FIXED 122097

ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setTextDecorationProperty

https://bugs.webkit.org/show_bug.cgi?id=122097

Summary ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setText...

Renata Hodovan

Reported 2013-09-30 04:15:49 PDT

Created attachment 212977 [details] Test case The failing test: <html> <body style="text-decoration: underline !important;">foo <iframe onload=" { document.designMode='on'; document.execCommand('selectall'); document.execCommand('RemoveFormat'); } "> </body> </html> The backtrace: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff56134c6 in WTFCrash () at /home/reni/Data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:342 342 *(int *)(uintptr_t)0xbbadbeef = 0; (gdb) bt #0 0x00007ffff56134c6 in WTFCrash () at /home/reni/Data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:342 #1 0x00007ffff41b9766 in WebCore::setTextDecorationProperty (style=0x89fd50, newTextDecoration=0x8e2cc0, propertyID=WebCore::CSSPropertyTextDecoration) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/EditingStyle.cpp:1408 #2 0x00007ffff41b9a67 in WebCore::StyleChange::extractTextStyles (this=0x7fffffff9630, document=0x89da80, style=0x89fd50, shouldUseFixedFontDefaultSize=false) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/EditingStyle.cpp:1442 #3 0x00007ffff41b94a8 in WebCore::StyleChange::StyleChange (this=0x7fffffff9630, style=0x905810, position=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/EditingStyle.cpp:1387 #4 0x00007ffff4194d94 in WebCore::ApplyStyleCommand::addInlineStyleIfNeeded (this=0x8e1e60, style=0x905810, passedStart=..., passedEnd=..., addStyledElement=WebCore::ApplyStyleCommand::DoNotAddStyledElement) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:1407 #5 0x00007ffff4192506 in WebCore::ApplyStyleCommand::applyInlineStyleToPushDown (this=0x8e1e60, node=0x89fbc0, style=0x905810) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:1027 #6 0x00007ffff4192961 in WebCore::ApplyStyleCommand::pushDownInlineStyleAroundNode (this=0x8e1e60, style=0x8e1f90, targetNode=0x7f9600) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:1072 #7 0x00007ffff4192d97 in WebCore::ApplyStyleCommand::removeInlineStyle (this=0x8e1e60, style=0x8e1f90, start=..., end=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:1106 #8 0x00007ffff418fd3c in WebCore::ApplyStyleCommand::applyInlineStyle (this=0x8e1e60, style=0x8e1f90) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:632 #9 0x00007ffff418d12b in WebCore::ApplyStyleCommand::doApply (this=0x8e1e60) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/ApplyStyleCommand.cpp:225 #10 0x00007ffff419d414 in WebCore::CompositeEditCommand::applyCommandToComposite (this=0x8f8ff0, prpCommand=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/CompositeEditCommand.cpp:264 #11 0x00007ffff41fe577 in WebCore::RemoveFormatCommand::doApply (this=0x8f8ff0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/RemoveFormatCommand.cpp:94 #12 0x00007ffff419d1d4 in WebCore::CompositeEditCommand::apply (this=0x8f8ff0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/CompositeEditCommand.cpp:213 #13 0x00007ffff419cfd4 in WebCore::applyCommand (command=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/CompositeEditCommand.cpp:172 #14 0x00007ffff41c00b9 in WebCore::Editor::removeFormattingAndStyle (this=0x794c20) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/Editor.cpp:698 #15 0x00007ffff41d20e5 in WebCore::executeRemoveFormat (frame=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/EditorCommand.cpp:981 #16 0x00007ffff41d3c71 in WebCore::Editor::Command::execute (this=0x7fffffff9ea0, parameter=..., triggeringEvent=0x0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/editing/EditorCommand.cpp:1717 #17 0x00007ffff409e848 in WebCore::Document::execCommand (this=0x89da80, commandName=..., userInterface=false, value=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:4174 #18 0x00007ffff4dcfe99 in WebCore::jsDocumentPrototypeFunctionExecCommand (exec=0x7fff95fc0f50) at generated/JSDocument.cpp:2763 #19 0x00007fff9ffff0e5 in ?? () #20 0x00007fffffffa050 in ?? () #21 0x00007ffff674749c in llint_op_call () from /home/reni/Data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libQt5WebKit.so.5 #22 0x00007fff95fc0f98 in ?? () #23 0x0000000000761fc8 in ?? () #24 0x00007fffffffa010 in ?? () #25 0x00007ffff541de2f in JSC::JSStack::installTrapsAfterFrame (this=0x0, frame=0x0) at /home/reni/Data/REPOS/webkit_sec/Source/JavaScriptCore/interpreter/JSStackInlines.h:214 #26 0x00007ffff54311e2 in JSC::JITCode::execute (this=0x8ca580, stack=0x761fc8, callFrame=0x7fff95fc0f98, vm=0x80f650) at /home/reni/Data/REPOS/webkit_sec/Source/JavaScriptCore/jit/JITCode.cpp:46 #27 0x00007ffff541ac42 in JSC::Interpreter::executeCall (this=0x761fb0, callFrame=0x7fff9c17f9b0, function=0x7fff9c0de4b0, callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at /home/reni/Data/REPOS/webkit_sec/Source/JavaScriptCore/interpreter/Interpreter.cpp:963 #28 0x00007ffff5507035 in JSC::call (exec=0x7fff9c17f9b0, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at /home/reni/Data/REPOS/webkit_sec/Source/JavaScriptCore/runtime/CallData.cpp:39 #29 0x00007ffff3e1ccf2 in WebCore::JSMainThreadExecState::call (exec=0x7fff9c17f9b0, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/bindings/js/JSMainThreadExecState.h:53 #30 0x00007ffff3e48e37 in WebCore::JSEventListener::handleEvent (this=0x776f20, scriptExecutionContext=0x89db30, event=0x8b8450) ---Type <return> to continue, or q <return> to quit--- at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/bindings/js/JSEventListener.cpp:133 #31 0x00007ffff410c31a in WebCore::EventTarget::fireEventListeners (this=0x89fbc0, event=0x8b8450, d=0x776f90, entry=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventTarget.cpp:277 #32 0x00007ffff410c02d in WebCore::EventTarget::fireEventListeners (this=0x89fbc0, event=0x8b8450) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventTarget.cpp:233 #33 0x00007ffff41389b1 in WebCore::Node::handleLocalEvents (this=0x89fbc0, event=0x8b8450) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Node.cpp:2067 #34 0x00007ffff40fe5a8 in WebCore::EventContext::handleLocalEvents (this=0x8bd1d0, event=0x8b8450) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventContext.cpp:58 #35 0x00007ffff410049f in WebCore::EventDispatcher::dispatchEventAtTarget (this=0x7fffffffa6e0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventDispatcher.cpp:160 #36 0x00007ffff410015c in WebCore::EventDispatcher::dispatch (this=0x7fffffffa6e0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventDispatcher.cpp:117 #37 0x00007ffff40fef69 in WebCore::EventDispatchMediator::dispatchEvent (this=0x8bb850, dispatcher=0x7fffffffa6e0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventDispatchMediator.cpp:54 #38 0x00007ffff40ff748 in WebCore::EventDispatcher::dispatchEvent (node=0x89fbc0, mediator=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/EventDispatcher.cpp:51 #39 0x00007ffff4138bc6 in WebCore::Node::dispatchEvent (this=0x89fbc0, event=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Node.cpp:2088 #40 0x00007ffff452f025 in WebCore::DOMWindow::dispatchLoadEvent (this=0x8b9660) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/page/DOMWindow.cpp:1691 #41 0x00007ffff409cdf1 in WebCore::Document::dispatchWindowLoadEvent (this=0x8bd810) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:3647 #42 0x00007ffff4098412 in WebCore::Document::implicitClose (this=0x8bd810) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:2440 #43 0x00007ffff44a9053 in WebCore::FrameLoader::checkCallImplicitClose (this=0x8a0210) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:850 #44 0x00007ffff44a8de7 in WebCore::FrameLoader::checkCompleted (this=0x8a0210) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:793 #45 0x00007ffff44a8b42 in WebCore::FrameLoader::finishedParsing (this=0x8a0210) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:726 #46 0x00007ffff409f62b in WebCore::Document::finishedParsing (this=0x8bd810) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:4439 #47 0x00007ffff42fcd9b in WebCore::HTMLConstructionSite::finishedParsing (this=0x8bcab8) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:352 #48 0x00007ffff43338f3 in WebCore::HTMLTreeBuilder::finished (this=0x8bcaa0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2908 #49 0x00007ffff43046ec in WebCore::HTMLDocumentParser::end (this=0x8bbe70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:758 #50 0x00007ffff43047d7 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x8bbe70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:769 #51 0x00007ffff430341f in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x8bbe70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:212 #52 0x00007ffff430481c in WebCore::HTMLDocumentParser::attemptToEnd (this=0x8bbe70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:781 #53 0x00007ffff43048d5 in WebCore::HTMLDocumentParser::finish (this=0x8bbe70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:830 #54 0x00007ffff44a0792 in WebCore::DocumentWriter::end (this=0x8b9d70) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentWriter.cpp:245 #55 0x00007ffff4492d46 in WebCore::DocumentLoader::finishedLoading (this=0x8b9cd0, finishTime=9378.8878862140009) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:408 #56 0x00007ffff44967e5 in WebCore::DocumentLoader::maybeLoadEmpty (this=0x8b9cd0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:1347 #57 0x00007ffff44968f8 in WebCore::DocumentLoader::startLoadingMainResource (this=0x8b9cd0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:1359 #58 0x00007ffff44af122 in WebCore::FrameLoader::continueLoadAfterWillSubmitForm (this=0x8a0210) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:2230 #59 0x00007ffff44b1ddc in WebCore::FrameLoader::continueLoadAfterNavigationPolicy (this=0x8a0210, formState=..., shouldContinue=true) ---Type <return> to continue, or q <return> to quit--- at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:2884 #60 0x00007ffff44b1275 in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy (argument=0x8a0210, request=..., formState=..., shouldContinue=true) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:2714 #61 0x00007ffff44ca498 in WebCore::PolicyCallback::call (this=0x7fffffffb510, shouldContinue=true) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/PolicyCallback.cpp:103 #62 0x00007ffff44cb5c5 in WebCore::PolicyChecker::continueAfterNavigationPolicy (this=0x8a04c0, policy=WebCore::PolicyUse) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/PolicyChecker.cpp:180 #63 0x00007ffff3a5c3f3 in WebCore::FrameLoaderClientQt::callPolicyFunction (this=0x8b44e0, function= (void (WebCore::PolicyChecker::*)(WebCore::PolicyChecker * const, WebCore::PolicyAction)) 0x7ffff44cb37a <WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)>, action=WebCore::PolicyUse) at /home/reni/Data/REPOS/webkit_sec/Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:246 #64 0x00007ffff3a622b0 in WebCore::FrameLoaderClientQt::dispatchDecidePolicyForNavigationAction (this=0x8b44e0, function= (void (WebCore::PolicyChecker::*)(WebCore::PolicyChecker * const, WebCore::PolicyAction)) 0x7ffff44cb37a <WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)>, action=..., request=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1287 #65 0x00007ffff44caec6 in WebCore::PolicyChecker::checkNavigationPolicy (this=0x8a04c0, request=..., loader=0x8b9cd0, formState=..., function=0x7ffff44b1226 <WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>, argument=0x8a0210) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/PolicyChecker.cpp:99 #66 0x00007ffff44abd03 in WebCore::FrameLoader::loadWithDocumentLoader (this=0x8a0210, loader=0x8b9cd0, type=WebCore::FrameLoadTypeRedirectWithLockedBackForwardList, prpFormState=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:1423 #67 0x00007ffff44ab5e3 in WebCore::FrameLoader::loadWithNavigationAction (this=0x8a0210, request=..., action=..., lockHistory=false, type=WebCore::FrameLoadTypeRedirectWithLockedBackForwardList, formState=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:1327 #68 0x00007ffff44aac6f in WebCore::FrameLoader::loadURL (this=0x8a0210, newURL=..., referrer=..., frameName=..., lockHistory=false, newLoadType=WebCore::FrameLoadTypeRedirectWithLockedBackForwardList, event=..., prpFormState=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:1262 #69 0x00007ffff44a9315 in WebCore::FrameLoader::loadURLIntoChildFrame (this=0x770e60, url=..., referer=..., childFrame=0x8a0180) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/FrameLoader.cpp:878 #70 0x00007ffff3a626fe in WebCore::FrameLoaderClientQt::createFrame (this=0x7adef0, url=..., name=..., ownerElement=0x89fbc0, referrer=..., allowsScrolling=true, marginWidth=-1, marginHeight=-1) at /home/reni/Data/REPOS/webkit_sec/Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1332 #71 0x00007ffff44db07c in WebCore::SubframeLoader::loadSubframe (this=0x771210, ownerElement=0x89fbc0, url=..., name=..., referrer=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/SubframeLoader.cpp:361 #72 0x00007ffff44dae11 in WebCore::SubframeLoader::loadOrRedirectSubframe (this=0x771210, ownerElement=0x89fbc0, url=..., frameName=..., lockHistory=true, lockBackForwardList=true) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/SubframeLoader.cpp:335 #73 0x00007ffff44d9ac7 in WebCore::SubframeLoader::requestFrame (this=0x771210, ownerElement=0x89fbc0, urlString=..., frameName=..., lockHistory=true, lockBackForwardList=true) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/SubframeLoader.cpp:89 #74 0x00007ffff4291708 in WebCore::HTMLFrameElementBase::openURL (this=0x89fbc0, lockHistory=true, lockBackForwardList=true) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:89 #75 0x00007ffff4291b7c in WebCore::HTMLFrameElementBase::setNameAndOpenURL (this=0x89fbc0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:142 #76 0x00007ffff4291c47 in WebCore::HTMLFrameElementBase::didNotifySubtreeInsertions (this=0x89fbc0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/HTMLFrameElementBase.cpp:173 #77 0x00007ffff4082402 in WebCore::ChildNodeInsertionNotifier::notify (this=0x7fffffffc9a0, node=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/ContainerNodeAlgorithms.h:234 #78 0x00007ffff40853f5 in WebCore::ContainerNode::parserAppendChild (this=0x7f9550, newChild=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/ContainerNode.cpp:764 #79 0x00007ffff42fb94a in WebCore::executeTask (task=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:97 #80 0x00007ffff42fbd6f in WebCore::HTMLConstructionSite::executeQueuedTasks (this=0x79cec8) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLConstructionSite.cpp:150 #81 0x00007ffff4327382 in WebCore::HTMLTreeBuilder::constructTree (this=0x79ceb0, token=0x7fffffffcb10) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:368 #82 0x00007ffff43040de in WebCore::HTMLDocumentParser::constructTreeFromHTMLToken (this=0x76d990, rawToken=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:595 ---Type <return> to continue, or q <return> to quit--- #83 0x00007ffff4303d49 in WebCore::HTMLDocumentParser::pumpTokenizer (this=0x76d990, mode=WebCore::HTMLDocumentParser::AllowYield) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:552 #84 0x00007ffff4303539 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x76d990, mode=WebCore::HTMLDocumentParser::AllowYield) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:236 #85 0x00007ffff430461f in WebCore::HTMLDocumentParser::append (this=0x76d990, inputSource=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/html/parser/HTMLDocumentParser.cpp:742 #86 0x00007ffff408be6a in WebCore::DecodedDataDocumentParser::flush (this=0x76d990, writer=0x72f590) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/DecodedDataDocumentParser.cpp:60 #87 0x00007ffff44a075f in WebCore::DocumentWriter::end (this=0x72f590) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentWriter.cpp:242 #88 0x00007ffff4492d46 in WebCore::DocumentLoader::finishedLoading (this=0x72f4f0, finishTime=0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:408 #89 0x00007ffff4492ab4 in WebCore::DocumentLoader::notifyFinished (this=0x72f4f0, resource=0x72fd80) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/DocumentLoader.cpp:345 #90 0x00007ffff4479bcc in WebCore::CachedResource::checkNotify (this=0x72fd80) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedResource.cpp:369 #91 0x00007ffff4479ca6 in WebCore::CachedResource::finishLoading (this=0x72fd80) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedResource.cpp:385 #92 0x00007ffff4476360 in WebCore::CachedRawResource::finishLoading (this=0x72fd80, data=0x7671f0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/cache/CachedRawResource.cpp:94 #93 0x00007ffff44dcc2d in WebCore::SubresourceLoader::didFinishLoading (this=0x774730, finishTime=0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/SubresourceLoader.cpp:283 #94 0x00007ffff44d34e7 in WebCore::ResourceLoader::didFinishLoading (this=0x774730, finishTime=0) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/loader/ResourceLoader.cpp:489 #95 0x00007ffff49954b5 in WebCore::QNetworkReplyHandler::finish (this=0x74cd20) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:516 #96 0x00007ffff49940dd in WebCore::QNetworkReplyHandlerCallQueue::flush (this=0x74cd58) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:250 #97 0x00007ffff4993ddb in WebCore::QNetworkReplyHandlerCallQueue::push (this=0x74cd58, method= (void (WebCore::QNetworkReplyHandler::*)(WebCore::QNetworkReplyHandler * const)) 0x7ffff49952fa <WebCore::QNetworkReplyHandler::finish()>) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:216 #98 0x00007ffff4994da8 in WebCore::QNetworkReplyWrapper::didReceiveFinished (this=0x7b1c30) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp:409 #99 0x00007ffff4997728 in WebCore::QNetworkReplyWrapper::qt_static_metacall (_o=0x7b1c30, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x7fffffffd230) at .moc/release-shared/moc_QNetworkReplyHandler.cpp:175 #100 0x00007ffff1d9ed71 in QMetaObject::activate(QObject*, int, int, void**) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #101 0x00007ffff1da033e in QObject::event(QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #102 0x00007ffff2c6ea24 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Widgets.so.5 #103 0x00007ffff2c71eb6 in QApplication::notify(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Widgets.so.5 #104 0x00007ffff1d778f4 in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #105 0x00007ffff1d7a1a9 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #106 0x00007ffff1dc19c3 in ?? () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #107 0x00007fffeeb88d53 in g_main_dispatch (context=0x656e00) at /build/buildd/glib2.0-2.32.3/./glib/gmain.c:2539 #108 g_main_context_dispatch (context=0x656e00) at /build/buildd/glib2.0-2.32.3/./glib/gmain.c:3075 #109 0x00007fffeeb890a0 in g_main_context_iterate (dispatch=1, block=<optimized out>, context=0x656e00, self=<optimized out>) at /build/buildd/glib2.0-2.32.3/./glib/gmain.c:3146 #110 g_main_context_iterate (context=0x656e00, block=<optimized out>, dispatch=1, self=<optimized out>) at /build/buildd/glib2.0-2.32.3/./glib/gmain.c:3083 #111 0x00007fffeeb89164 in g_main_context_iteration (context=0x656e00, may_block=1) at /build/buildd/glib2.0-2.32.3/./glib/gmain.c:3207 #112 0x00007ffff1dc1e04 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #113 0x00007ffff1d7668b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 #114 0x00007ffff1d7a6de in QCoreApplication::exec() () from /usr/local/Trolltech/Qt5/Qt-5.1.1/lib/libQt5Core.so.5 ---Type <return> to continue, or q <return> to quit--- #115 0x0000000000420da0 in launcherMain (app=...) at /home/reni/Data/REPOS/webkit_sec/Tools/QtTestBrowser/qttestbrowser.cpp:50 #116 0x0000000000422880 in main (argc=2, argv=0x7fffffffdf08) at /home/reni/Data/REPOS/webkit_sec/Tools/QtTestBrowser/qttestbrowser.cpp:319

Attachments
Test case (272 bytes, text/html) 2013-09-30 04:15 PDT, Renata Hodovan	no flags	Details
workinprogress (4.44 KB, patch) 2013-10-13 13:40 PDT, Santosh Mahto	no flags	Details Formatted Diff Diff
Patch (6.29 KB, patch) 2013-10-14 06:32 PDT, Santosh Mahto	no flags	Details Formatted Diff Diff
Another testcase(bit less confusing) (315 bytes, text/html) 2013-10-16 10:34 PDT, Santosh Mahto	no flags	Details
Patch (6.57 KB, patch) 2013-10-17 12:01 PDT, Santosh Mahto	no flags	Details Formatted Diff Diff
Patch (6.22 KB, patch) 2013-10-19 04:49 PDT, Santosh Mahto	no flags	Details Formatted Diff Diff
Patch (5.96 KB, patch) 2013-10-20 05:43 PDT, Santosh Mahto	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Santosh Mahto

Comment 1 2013-10-13 13:40:23 PDT

Created attachment 214109 [details] workinprogress

Santosh Mahto

Comment 2 2013-10-14 06:32:07 PDT

Created attachment 214148 [details] Patch

Santosh Mahto

Comment 3 2013-10-14 06:38:46 PDT

There is two bug in the crash. 1. Actual assert is probably useless. on remove-format command we need to remove the property even if it is declared as !important. 2. second pushing down style on iframe element. it is done in same way as for other inline element. adding styled node around inline element.But this triggers looping in onload event call, as new node are inserted in iframe which again triggers loadframe and again onload eventhandler are called. So pushing down style on iframe element should be done by adding style attribute to iframe element. I uploaded the patch which solves the crash, Please review....

Ryosuke Niwa

Comment 4 2013-10-14 09:32:08 PDT

Comment on attachment 214148 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214148&action=review > Source/WebCore/ChangeLog:25 > + (WebCore::StyleChange::extractTextStyles): Remove property even if > + it is declared !important. Why? > Source/WebCore/editing/EditingStyle.cpp:1446 > - setTextDecorationProperty(style, newTextDecoration.get(), CSSPropertyTextDecoration); > + if (newTextDecoration->length()) > + setTextDecorationProperty(style, newTextDecoration.get(), CSSPropertyTextDecoration); > + else > + style->removeProperty(CSSPropertyTextDecoration); > + This doesn't make any sense. The whole point of setTextDecorationProperty is to take care of that. We should figure out why setTextDecorationProperty has the assertion instead of working around it here. > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:6 > + <iframe onload=" { document.designMode='on'; > + document.execCommand('selectall'); > + document.execCommand('RemoveFormat'); } "> Please fix the indentation. Also, we should use dump-as-text or dump-as-markup. There is no reason for this test to be a render tree dump. r- because of this.

Santosh Mahto

Comment 5 2013-10-16 10:34:02 PDT

Created attachment 214371 [details] Another testcase(bit less confusing) Bit simple test case . Try as it is OR remove contenteditable from <html contenteditable> and uncomment designmode. Both case it will crash in setTextDecorationproperty.

Santosh Mahto

Comment 6 2013-10-16 10:34:59 PDT

New test case code. <!DOCTYPE html> <html contenteditable> <body style="text-decoration: underline !important;"><p>dasda</p>foo <span >santosh</span> <script> // document.designMode='on'; document.execCommand('selectall'); document.execCommand('RemoveFormat', false, null); </script> </body> </html>

Santosh Mahto

Comment 7 2013-10-16 11:23:59 PDT

Editing bugs are so tricky!! (In reply to comment #4) > (From update of attachment 214148 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214148&action=review > > > Source/WebCore/ChangeLog:25 > > + (WebCore::StyleChange::extractTextStyles): Remove property even if > > + it is declared !important. > > Why? Yes becasue we do it for other styleproperty in same function. In code ApplyStyleCommand::addInlineStyleIfNeeded : we try to find StyleChange that we need to apply(pushing down style in conflicting style node childs). StyleChange class constructor doesnot consider !important in its calculation for any other property.it just try to find 6 text changes to applied. : m_applyBold(false) , m_applyItalic(false) , m_applyUnderline(false) , m_applyLineThrough(false) , m_applySubscript(false) , m_applySuperscript(false) What I mean is these text properties are applied in different way (by surrounding with node <b>, <i> etc) not by CSS rules to child node. So we need to clear out css style (style->removeProperty()) whether it is declared !important or not. !important property is used in resolving the style among multiple CSS contenders. since for textdecoration:underline !important we will enforce style by adding node(utag) so !important consideration is useless. > > Source/WebCore/editing/EditingStyle.cpp:1446 > > - setTextDecorationProperty(style, newTextDecoration.get(), CSSPropertyTextDecoration); > > + if (newTextDecoration->length()) > > + setTextDecorationProperty(style, newTextDecoration.get(), CSSPropertyTextDecoration); > > + else > > + style->removeProperty(CSSPropertyTextDecoration); > > + > > This doesn't make any sense. The whole point of setTextDecorationProperty is to take care of that. > We should figure out why setTextDecorationProperty has the assertion instead of working around it here. As above explanation: since textdecoration:underline style will be applied by surrounding node and so !important is not relevant. So We should remove property[ style->removeProperty(propertyID);](if empty) even if it is declared as important in code. So in this path of flow ASSERT is wrong. I think better way is to remove ASSERT from setTextDecorationProperty. > > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:6 > > + <iframe onload=" { document.designMode='on'; > > + document.execCommand('selectall'); > > + document.execCommand('RemoveFormat'); } "> > > Please fix the indentation. > Also, we should use dump-as-text or dump-as-markup. > There is no reason for this test to be a render tree dump. > r- because of this. I will update updated testcase in next patch, thanks There is more to explain: Actually crash is Quite general(see next test case). code modification in iframe: As you can see in patch I added an iframe check. The reason for this was that since we apply style on inline block by surrounding with node(utag, <b>), To do this we disconnect iframe node and again reattach as child of styling node.But whenever iframe node is added on tree, subframe loading is triggered and again onload event handler will be called (1st test case) and again everything will repeat and finally stackoverflow. But I think node->renderer()->isReplaced() will be a better check there. please comment if i confused you anywhere...

Santosh Mahto

Comment 8 2013-10-17 12:01:08 PDT

Created attachment 214482 [details] Patch

Ryosuke Niwa

Comment 9 2013-10-18 13:45:24 PDT

Comment on attachment 214482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214482&action=review > Source/WebCore/editing/ApplyStyleCommand.cpp:1016 > - if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) { > + if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount() || node->renderer()->isReplaced()) && node->isHTMLElement()) { All replaced elements? So we do this for img, input, etc...? That doesn't sound right. We also need tests for that. > Source/WebCore/editing/EditingStyle.cpp:-1411 > // text-decoration: none is redundant since it does not remove any text decorations. > - ASSERT(!style->propertyIsImportant(propertyID)); I think the point of this code is that if text-decoration was specified with !important then we don't want to be overriding it with a new text decoration. We should add a bunch of test cases for that. > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:7 > + <iframe onload="{ document.designMode='on'; > + document.execCommand('selectall'); > + document.execCommand('RemoveFormat'); }"></iframe> Can we move this function out of the attribute and put it in the script element below?

Santosh Mahto

Comment 10 2013-10-18 23:50:56 PDT

(In reply to comment #9) > (From update of attachment 214482 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214482&action=review > > > Source/WebCore/editing/ApplyStyleCommand.cpp:1016 > > - if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) { > > + if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount() || node->renderer()->isReplaced()) && node->isHTMLElement()) { > > All replaced elements? So we do this for img, input, etc...? That doesn't sound right. > We also need tests for that. You are probably right.., considering the domain of bug it not right to change the behavior for all replaced element. I am planning to stick to just iframe check. > > Source/WebCore/editing/EditingStyle.cpp:-1411 > > // text-decoration: none is redundant since it does not remove any text decorations. > > - ASSERT(!style->propertyIsImportant(propertyID)); > > I think the point of this code is that if text-decoration was specified with !important then we don't want to be overriding it with a new text decoration. The point is new text decoration at this point will be empty. if (newTextDecoration->length()) { } else { ASSERT(!style->propertyIsImportant(propertyID)); style->removeProperty(propertyID); } > We should add a bunch of test cases for that. > > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:7 > > + <iframe onload="{ document.designMode='on'; > > + document.execCommand('selectall'); > > + document.execCommand('RemoveFormat'); }"></iframe> > > Can we move this function out of the attribute and put it in the script element below?

Santosh Mahto

Comment 11 2013-10-19 01:03:45 PDT

(In reply to comment #9) > (From update of attachment 214482 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214482&action=review > > > Source/WebCore/editing/ApplyStyleCommand.cpp:1016 > > - if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount()) && node->isHTMLElement()) { > > + if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount() || node->renderer()->isReplaced()) && node->isHTMLElement()) { > > All replaced elements? So we do this for img, input, etc...? That doesn't sound right. > We also need tests for that. You are probably right.., considering the domain of bug it not right to change the behavior for all replaced element. I am planning to stick to just iframe check. > > Source/WebCore/editing/EditingStyle.cpp:-1411 > > // text-decoration: none is redundant since it does not remove any text decorations. > > - ASSERT(!style->propertyIsImportant(propertyID)); > > I think the point of this code is that if text-decoration was specified with !important then we don't want to be overriding it with a new text decoration. The point is new text decoration at this point will be empty. So we remove the property. it does not matter its important or not. if (newTextDecoration->length()) { } else { // newTextDecoration->length() == 0 ASSERT(!style->propertyIsImportant(propertyID)); style->removeProperty(propertyID); } > > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:7 > > + <iframe onload="{ document.designMode='on'; > > + document.execCommand('selectall'); > > + document.execCommand('RemoveFormat'); }"></iframe> > > Can we move this function out of the attribute and put it in the script element below? Yes I will do.

Santosh Mahto

Comment 12 2013-10-19 04:49:54 PDT

Created attachment 214648 [details] Patch

Santosh Mahto

Comment 13 2013-10-19 05:12:12 PDT

> We also need tests for that. > > > Source/WebCore/editing/EditingStyle.cpp:-1411 > > // text-decoration: none is redundant since it does not remove any text decorations. > > - ASSERT(!style->propertyIsImportant(propertyID)); > > I think the point of this code is that if text-decoration was specified with !important then we don't want to be overriding it with a new text decoration. > We should add a bunch of test cases for that. But here we should always override as per current flow. Here we are not allowing removing of text property if it is declared !important.this way we would be end applying text decoration in two way simultaneously(one by style property and other by utag) Regarding removal of ASSERT in setTextDecorationProperty(): setTextDecorationProperty() is private function and called from two places in EditingStyle Class. I checked the both scenarios and found ASSERT is wrong in setTextDecorationProperty(). There is no meaning of !important if new text decoration value is empty in Editing scope.

Ryosuke Niwa

Comment 14 2013-10-19 07:39:53 PDT

(In reply to comment #13) > > We also need tests for that. > > > > > Source/WebCore/editing/EditingStyle.cpp:-1411 > > > // text-decoration: none is redundant since it does not remove any text decorations. > > > - ASSERT(!style->propertyIsImportant(propertyID)); > > > > I think the point of this code is that if text-decoration was specified with !important then we don't want to be overriding it with a new text decoration. > > We should add a bunch of test cases for that. > But here we should always override as per current flow. > Here we are not allowing removing of text property if it is declared !important.this way we would be end applying text decoration in two way simultaneously(one by style property and other by utag) > > Regarding removal of ASSERT in setTextDecorationProperty(): > > setTextDecorationProperty() is private function and called from two places in EditingStyle Class. I checked the both scenarios and found ASSERT is wrong in setTextDecorationProperty(). There is no meaning of !important if new text decoration value is empty in Editing scope. Are you use neither function is called while pushing style in ApplyStyleCommand? The problem here is that even if new text-decoration was empty or some other value, we shouldn't be overriding a property with !important. Although we don't do a good job dealing with !important anyway so there is an argument to be made about ignoring !important in some cases. Regardless, we need a lot more test cases involing !important in applying & removing inline styles.

Santosh Mahto

Comment 15 2013-10-19 10:24:37 PDT

ApplyStyleCommand? The problem here is that even if new text-decoration was empty or some other value, we shouldn't be overriding a property with !important. > > Although we don't do a good job dealing with !important anyway so there is an argument to be made about ignoring !important in some cases. > > Regardless, we need a lot more test cases involing !important in applying & removing inline styles. I got your concerns. The !important handling is really debatable here. Although changes look ok to me (with overriding issue). But really I need to write more test cases covering the inline style handling scenario and to confirm about safety. Thanks for your points...

Ryosuke Niwa

Comment 16 2013-10-19 17:35:44 PDT

Comment on attachment 214648 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214648&action=review > Source/WebCore/editing/ApplyStyleCommand.cpp:1016 > + if ((node->renderer()->isRenderBlockFlow() || node->childNodeCount() || node->hasTagName(iframeTag)) && node->isHTMLElement()) { On my second thought, it might make more sense to simply ignore iframe because adding inline style on iframe should have no effect. e.g. underlining, bolding, etc… iframe doesn't make any sense since the said inline style won't be applied to the content inside iframe. Although if iframe's fallback content were to be shown, perhaps… > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:7 > + document.designMode='on'; Nit: Space around =. > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:8 > + document.execCommand('selectall'); Why don't we also capitalize SelectAll? > LayoutTests/editing/execCommand/remove-format-textdecoration-in-iframe.html:15 > + Markup.dump('container'); Since the style is on body, it doesn't really help to dump the body. Can we add a wrapping node inside body with text-decoration? If that doesn't work, it might make sense to dump the whole html after removing script elements (to avoid cluttering).

Santosh Mahto

Comment 17 2013-10-20 05:43:25 PDT

Created attachment 214696 [details] Patch

WebKit Commit Bot

Comment 18 2013-10-21 00:09:24 PDT

Comment on attachment 214696 [details] Patch Clearing flags on attachment: 214696 Committed r157710: <http://trac.webkit.org/changeset/157710>

WebKit Commit Bot

Comment 19 2013-10-21 00:09:27 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component HTML Editing

Assignee

Nobody

Reported

2013-09-30 04:15 PDT

Modified

2013-10-21 00:09 PDT History

CC List

4 users Show

URL

Keywords

Depends on

Blocks

116980

Dependencies

tree graph