If you specify any counter, { content: counter(dummy,disc); }, on a code tag seems to crash the browser. The crash does not occur on other tags. Stable Safari does not crash, but the counters do not show up either.
Created attachment 10955 [details] Reduction
Thread 0 Crashed: 0 com.apple.WebCore 0x01e0c9bc WebCore::StringImpl::length() const + 20 (StringImpl.h:74) 1 com.apple.WebCore 0x01b10dbc WebCore::RenderText::allAscii() const + 228 (RenderText.cpp:446) 2 com.apple.WebCore 0x01b10e5c WebCore::RenderText::shouldUseMonospaceCache(WebCore::Font const*) const + 72 (RenderText.cpp:459) 3 com.apple.WebCore 0x01b10f60 WebCore::RenderText::cacheWidths() + 56 (RenderText.cpp:468) 4 com.apple.WebCore 0x01b13a08 WebCore::RenderText::setStyle(WebCore::RenderStyle*) + 532 (RenderText.cpp:128) 5 com.apple.WebCore 0x01acecb4 WebCore::RenderContainer::updatePseudoChildForObject(WebCore::RenderStyle::PseudoId, WebCore::RenderObject*) + 2460 (RenderContainer.cpp:356) 6 com.apple.WebCore 0x01acede8 WebCore::RenderContainer::updatePseudoChild(WebCore::RenderStyle::PseudoId) + 168 (RenderContainer.cpp:250) 7 com.apple.WebCore 0x01adea84 WebCore::RenderInline::setStyle(WebCore::RenderStyle*) + 356 (RenderInline.cpp:68) 8 com.apple.WebCore 0x01bdff1c WebCore::Node::createRendererIfNeeded() + 684 (Node.cpp:902) 9 com.apple.WebCore 0x01be787c WebCore::Element::attach() + 44 (Element.cpp:560) 10 com.apple.WebCore 0x018f46a0 WebCore::HTMLParser::insertNode(WebCore::Node*, bool) + 680 (HTMLParser.cpp:288) 11 com.apple.WebCore 0x018f616c WebCore::HTMLParser::parseToken(WebCore::Token*) + 1388 (HTMLParser.cpp:220) 12 com.apple.WebCore 0x018f9cb4 WebCore::HTMLTokenizer::processToken() + 624 (HTMLTokenizer.cpp:1645) 13 com.apple.WebCore 0x018fd87c WebCore::HTMLTokenizer::parseTag(WebCore::SegmentedString&, WebCore::HTMLTokenizer::State) + 7252 (HTMLTokenizer.cpp:1218) 14 com.apple.WebCore 0x018fe384 WebCore::HTMLTokenizer::write(WebCore::SegmentedString const&, bool) + 1444 (HTMLTokenizer.cpp:1444) 15 com.apple.WebCore 0x019fe9fc WebCore::Frame::write(char const*, int) + 1200 (Frame.cpp:717) 16 com.apple.WebCore 0x019f3014 WebCore::Frame::addData(char const*, int) + 340 (Frame.cpp:2787) 17 com.apple.WebCore 0x01a3f1e8 -[WebCoreFrameBridge addData:] + 224 (WebCoreFrameBridge.mm:587) 18 com.apple.WebKit 0x003351ac -[WebFrameBridge receivedData:textEncodingName:] + 276 (WebFrameBridge.m:502) 19 com.apple.WebKit 0x0035365c -[WebHTMLRepresentation receivedData:withDataSource:] + 268 (WebHTMLRepresentation.m:150) 20 com.apple.WebKit 0x0033f28c -[WebDataSource(WebInternal) _receivedData:] + 108 (WebDataSource.m:171) 21 com.apple.WebKit 0x003d7878 -[WebFrameLoader committedLoadWithDocumentLoadState:data:] + 120 (WebFrameLoader.m:987) 22 com.apple.WebKit 0x003e4704 -[WebDocumentLoadState commitLoadWithData:] + 140 (WebDocumentLoadState.m:299) 23 com.apple.WebKit 0x003e489c -[WebDocumentLoadState receivedData:] + 168 (WebDocumentLoadState.m:313) 24 com.apple.WebKit 0x003d4da0 -[WebFrameLoader _receivedData:] + 104 (WebFrameLoader.m:561) 25 com.apple.WebKit 0x003dd2d0 -[WebMainResourceLoader addData:allAtOnce:] + 156 (WebMainResourceLoader.m:152) 26 com.apple.WebKit 0x003db858 -[WebLoader didReceiveData:lengthReceived:allAtOnce:] + 128 (WebLoader.m:366) 27 com.apple.WebKit 0x003de410 -[WebMainResourceLoader didReceiveData:lengthReceived:allAtOnce:] + 492 (WebMainResourceLoader.m:351) 28 com.apple.WebKit 0x003dc1a4 -[WebLoader connection:didReceiveData:lengthReceived:] + 192 (WebLoader.m:466) 29 com.apple.Foundation 0x929935d4 -[NSURLConnection(NSURLConnectionInternal) _sendDidReceiveDataCallback] + 564 30 com.apple.Foundation 0x92991a74 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 488 31 com.apple.Foundation 0x92991810 _sendCallbacks + 156 32 com.apple.CoreFoundation 0x907dd4cc __CFRunLoopDoSources0 + 384 33 com.apple.CoreFoundation 0x907dc9fc __CFRunLoopRun + 452 34 com.apple.CoreFoundation 0x907dc47c CFRunLoopRunSpecific + 268 35 com.apple.HIToolbox 0x93208740 RunCurrentEventLoopInMode + 264 36 com.apple.HIToolbox 0x93207dd4 ReceiveNextEventCommon + 380 37 com.apple.HIToolbox 0x93207c40 BlockUntilNextEventMatchingListInMode + 96 38 com.apple.AppKit 0x9370bae4 _DPSNextEvent + 384 39 com.apple.AppKit 0x9370b7a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 40 com.apple.Safari 0x00006740 0x1000 + 22336 41 com.apple.AppKit 0x93707cec -[NSApplication run] + 472 42 com.apple.AppKit 0x937f887c NSApplicationMain + 452 43 com.apple.Safari 0x0005c77c 0x1000 + 374652 44 com.apple.Safari 0x0005c624 0x1000 + 374308
4826798
Created attachment 12451 [details] Proposed patch A simple patch to resolve this issue. Another possible point for this patch would be not calling cacheWidths() from RenderText::setStyle if setText was not called. This is probably not a good spot however as this is not the only place setText may be called to change m_str's value.
(In reply to comment #4) > Created an attachment (id=12451) [edit] > Proposed patch > > A simple patch to resolve this issue. FWIW, this probably doesn't need to be a pixel test since it's testing non-crashing behavior. :)
(In reply to comment #5) > FWIW, this probably doesn't need to be a pixel test since it's testing > non-crashing behavior. :) Unless you're also testing the before/after rendering for <code> elements that's not included in other tests.
Comment on attachment 12451 [details] Proposed patch I believe a better fix is to fix allAscii() to work when m_str is 0. Just putting the for loop inside an if (m_str) would do the trick. But this patch is also OK as-is.
My patch removed the call to cacheWidths that was done after changing the text and moved it to setStyle instead. That's a problem since the value of shouldUseMonospaceCache is based on the value of allAscii. So I think we have a bug now when some all-ASCII text is replaced with text that has some non-ASCII characters in it. The monospace cache will be used in cases where it's not appropriate. Seems like we need to move the cacheWidths call back into setInternalString.
Created attachment 12590 [details] much larger patch, passes layout tests, I think we should land this one
Comment on attachment 12451 [details] Proposed patch Clearing the reviewed flag on this patch for now. Depending on feedback about my patch, might land this or mine.
Comment on attachment 12590 [details] much larger patch, passes layout tests, I think we should land this one \ No newline at end of file Please add a newline. + if (style()->collapseWhiteSpace() && next && !next->isBR() && next->isText() && static_cast<RenderText*>(next)->textLength() > 0) { The > 0 is unnecessary. + if (nextText->textLength() != 0) { Should be 'if (nextText->textLength())'. +++ WebKitTools/Scripts/run-webkit-tests (working copy) Doesn't belong in this patch. r=me w/o the run-webkit-tests changes.
Created attachment 12603 [details] Test case for the monospace cache regression
Committed revision 19027.