86077 2012-05-10 02:36:29 -0700 Viewport attribute values are leaked when document.write() is used. 2014-02-03 03:50:53 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED INVALID Qt, QtTriaged P1 Critical --- 79668 85425 1 ossy zalan hugo.lima kenneth kpiascik ossy tonikitoo zalan oldest_to_newest 620404 0 ossy 2012-05-10 02:36:29 -0700 The new assertion is introduced in r116571 and hit from the beginning. crash log for DumpRenderTree (pid 21943): STDOUT: <empty> STDERR: ASSERTION FAILED: m_viewportArguments.type == ViewportArguments::Implicit STDERR: ../../../../Source/WebCore/dom/Document.cpp(777) : void WebCore::Document::setDocType(WTF::PassRefPtr<WebCore::DocumentType>) STDERR: 1 0x7fce283a07c8 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::Document::setDocType(WTF::PassRefPtr<WebCore::DocumentType>)+0x11c) [0x7fce283a07c8] STDERR: 2 0x7fce283eff6d /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::DocumentType::insertedInto(WebCore::Node*)+0x13f) [0x7fce283eff6d] STDERR: 3 0x7fce28382258 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument(WebCore::Node*)+0x9a) [0x7fce28382258] STDERR: 4 0x7fce283823d7 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ChildNodeInsertionNotifier::notifyInsertedIntoDocument(WebCore::Node*)+0x23) [0x7fce283823d7] STDERR: 5 0x7fce28384e3f /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::ContainerNode::parserAddChild(WTF::PassRefPtr<WebCore::Node>)+0x14d) [0x7fce28384e3f] STDERR: 6 0x7fce28665d82 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(+0x25aed82) [0x7fce28665d82] STDERR: 7 0x7fce28666144 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLConstructionSite::executeQueuedTasks()+0x76) [0x7fce28666144] STDERR: 8 0x7fce28666f99 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLConstructionSite::insertDoctype(WebCore::AtomicHTMLToken&)+0x2b5) [0x7fce28666f99] STDERR: 9 0x7fce2868eeaa /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::processDoctypeToken(WebCore::AtomicHTMLToken&)+0x86) [0x7fce2868eeaa] STDERR: 10 0x7fce2868ed89 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken&)+0xa5) [0x7fce2868ed89] STDERR: 11 0x7fce2868ebe8 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken&)+0x50) [0x7fce2868ebe8] STDERR: 12 0x7fce2868eae2 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&)+0x66) [0x7fce2868eae2] STDERR: 13 0x7fce2866df3b /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)+0x327) [0x7fce2866df3b] STDERR: 14 0x7fce2866d801 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)+0xb1) [0x7fce2866d801] STDERR: 15 0x7fce2866e370 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::HTMLDocumentParser::insert(WebCore::SegmentedString const&)+0xa4) [0x7fce2866e370] STDERR: 16 0x7fce283a8b5a /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::Document::write(WebCore::SegmentedString const&, WebCore::Document*)+0x1ac) [0x7fce283a8b5a] STDERR: 17 0x7fce2811143e /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(+0x205a43e) [0x7fce2811143e] STDERR: 18 0x7fce281114dd /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::JSHTMLDocument::write(JSC::ExecState*)+0x33) [0x7fce281114dd] STDERR: 19 0x7fce29281fc7 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::jsHTMLDocumentPrototypeFunctionWrite(JSC::ExecState*)+0x12b) [0x7fce29281fc7] STDERR: 20 0x7fcddc620265 [0x7fcddc620265] 620415 1 zalan 2012-05-10 02:55:32 -0700 looking at it now 620492 2 zalan 2012-05-10 05:27:09 -0700 1, Document is reused multiple times by injecting new content using document.write(). 2, content A has xhtml-mobile dtd and document sets the legacy viewport attributes. 3, content B comes and assert hits because the viewport attribute type leaks from content A. it's good that the type check was replaced by the assert, as with the type check, we would have missed proper viewport attribute adaptation, while content A with 'meta viewport' -> content B with 'xhtml-mobile' transition. 620589 3 141173 zalan 2012-05-10 08:07:54 -0700 Created attachment 141173 test case 621333 4 ossy 2012-05-11 00:15:47 -0700 I skipped the asserting test - https://trac.webkit.org/changeset/116734 Please unskip it with the proper fix. 622884 5 zalan 2012-05-14 06:07:36 -0700 this gets a little bit more generic: given the following meta <meta id="viewport" name='viewport' content='width=device-width'> these 2 calls both update viewport values to default: document.getElementById("viewport").setAttribute("content", ""); document.getElementsByTagName("meta")[0].content=''; however, none of the calls below initiate viewport value update: a, var vp = document.getElementById("viewport"); vp.parentNode.removeChild(vp); b, document.getElementsByTagName("head")[0].innerHTML = ""; or even when the entire content of the document is trashed using document.open() -> document.write() Shouldn't dynamically removed meta viewport initiate update on the viewport values similarly when the 'content' attribute is changed? 622947 6 kenneth 2012-05-14 07:06:36 -0700 (In reply to comment #5) > this gets a little bit more generic: > > given the following meta > <meta id="viewport" name='viewport' content='width=device-width'> > > these 2 calls both update viewport values to default: > document.getElementById("viewport").setAttribute("content", ""); > document.getElementsByTagName("meta")[0].content=''; > > however, none of the calls below initiate viewport value update: > a, var vp = document.getElementById("viewport"); vp.parentNode.removeChild(vp); > b, document.getElementsByTagName("head")[0].innerHTML = ""; > > or even when the entire content of the document is trashed using > document.open() -> document.write() > > Shouldn't dynamically removed meta viewport initiate update on the viewport values similarly when the 'content' attribute is changed? This sounds sensible to me. 628349 7 tonikitoo 2012-05-18 21:19:44 -0700 (In reply to comment #6) > (In reply to comment #5) > > this gets a little bit more generic: > > > > given the following meta > > <meta id="viewport" name='viewport' content='width=device-width'> > > > > these 2 calls both update viewport values to default: > > document.getElementById("viewport").setAttribute("content", ""); > > document.getElementsByTagName("meta")[0].content=''; > > > > however, none of the calls below initiate viewport value update: > > a, var vp = document.getElementById("viewport"); vp.parentNode.removeChild(vp); > > b, document.getElementsByTagName("head")[0].innerHTML = ""; > > > > or even when the entire content of the document is trashed using > > document.open() -> document.write() > > > > Shouldn't dynamically removed meta viewport initiate update on the viewport values similarly when the 'content' attribute is changed? > > This sounds sensible to me. +1. how does firefox mobile or opera behavior with dynamic viewport values? 630355 8 zalan 2012-05-22 07:40:05 -0700 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > this gets a little bit more generic: > > > > > > given the following meta > > > <meta id="viewport" name='viewport' content='width=device-width'> > > > > > > these 2 calls both update viewport values to default: > > > document.getElementById("viewport").setAttribute("content", ""); > > > document.getElementsByTagName("meta")[0].content=''; > > > > > > however, none of the calls below initiate viewport value update: > > > a, var vp = document.getElementById("viewport"); vp.parentNode.removeChild(vp); > > > b, document.getElementsByTagName("head")[0].innerHTML = ""; > > > > > > or even when the entire content of the document is trashed using > > > document.open() -> document.write() > > > > > > Shouldn't dynamically removed meta viewport initiate update on the viewport values similarly when the 'content' attribute is changed? > > > > This sounds sensible to me. > > +1. how does firefox mobile or opera behavior with dynamic viewport values? FF mobile(14.0?) on Android(4.0.4) does not seem to be supporting any dynamic change of the viewport values (and it is broken in so many ways) However, Opera mobile(12.0) on Android(4.0.4) seems to be supporting changing the viewport values with 1, setAttribute, 2, removing the viewport using removeChild() -though it gets confused with multiple viewport metas. 3, emptying the <head> using document.getElementsByTagName("head")[0].innerHTML = ""; 4, and even using document.write() -though the new content's viewport values are ignored, so it looks like it just resets the viewport to default (as if no meta present) So Opera mobile clearly reacts on when the meta tag changes unlike us (other than the setAttribute()) 630366 9 zalan 2012-05-22 07:49:48 -0700 We need to be clever about this though as multiple viewport meta tags in the document can make things complicated, such as 1, changing the ignored ones should not have any impact on the viewport. 2, after removing the one was in use, some priority order need to be checked to see which one to take next. 3, adding a new one might or might not change the actual viewport values. (later when we introduce legacy tags to get converted into viewport values) 975828 10 jturcotte 2014-02-03 03:50:53 -0800 === Bulk closing of Qt bugs === If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it. If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines. 141173 2012-05-10 08:07:54 -0700 2012-05-10 08:07:54 -0700 test case xhtmlmobiledtd.html text/html 346 zalan PGh0bWw+CjxoZWFkPgogIDxtZXRhIG5hbWU9J3ZpZXdwb3J0JyBjb250ZW50PSd3aWR0aD1kZXZp Y2Utd2lkdGgnPgogIDxzY3JpcHQ+CiAgICAgZnVuY3Rpb24gZm9vKCkgewogICAgICAgZG9jdW1l bnQud3JpdGUoJzwhRE9DVFlQRSBodG1sIFBVQkxJQyAiLS8vV0FQRk9SVU0vL0RURCBYSFRNTCBN b2JpbGUgMS4xLy9FTiIgImh0dHA6Ly93d3cub3Blbm1vYmlsZWFsbGlhbmNlLm9yZy90ZWNoL0RU RC94aHRtbC1tb2JpbGUxMS5kdGQiPjxodG1sPjxib2R5PjwvYm9keT48L2h0bWw+Jyk7CiAgICAg fQogIDwvc2NyaXB0Pgo8L2hlYWQ+Cjxib2R5IG9ubG9hZD0nZm9vKCk7Jz4KPC9ib2R5Pgo8L2h0 bWw+Cg==