WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
11197
REGRESSION: Specifying a counter for a CODE tag's content style property on before or after causes a crash.
https://bugs.webkit.org/show_bug.cgi?id=11197
Summary
REGRESSION: Specifying a counter for a CODE tag's content style property on b...
Adam Elliot
Reported
2006-10-06 16:26:02 PDT
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.
Attachments
Reduction
(151 bytes, text/html)
2006-10-06 16:27 PDT
,
Adam Elliot
no flags
Details
Proposed patch
(21.57 KB, patch)
2007-01-15 03:45 PST
,
Andrew Wellington
no flags
Details
Formatted Diff
Diff
much larger patch, passes layout tests, I think we should land this one
(94.70 KB, patch)
2007-01-21 16:49 PST
,
Darin Adler
mitz: review+
Details
Formatted Diff
Diff
Test case for the monospace cache regression
(616 bytes, text/html)
2007-01-22 06:56 PST
,
mitz
no flags
Details
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Adam Elliot
Comment 1
2006-10-06 16:27:27 PDT
Created
attachment 10955
[details]
Reduction
mitz
Comment 2
2006-10-07 13:52:16 PDT
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
Stephanie Lewis
Comment 3
2006-11-08 14:41:05 PST
4826798
Andrew Wellington
Comment 4
2007-01-15 03:45:35 PST
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.
David Kilzer (:ddkilzer)
Comment 5
2007-01-15 07:27:03 PST
(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. :)
David Kilzer (:ddkilzer)
Comment 6
2007-01-15 07:50:47 PST
(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.
Darin Adler
Comment 7
2007-01-15 08:43:15 PST
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.
Darin Adler
Comment 8
2007-01-15 11:11:05 PST
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.
Darin Adler
Comment 9
2007-01-21 16:49:04 PST
Created
attachment 12590
[details]
much larger patch, passes layout tests, I think we should land this one
Darin Adler
Comment 10
2007-01-21 17:23:00 PST
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.
mitz
Comment 11
2007-01-21 23:33:39 PST
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.
mitz
Comment 12
2007-01-22 06:56:03 PST
Created
attachment 12603
[details]
Test case for the monospace cache regression
Darin Adler
Comment 13
2007-01-22 09:48:25 PST
Committed revision 19027.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug