Bug 32374

Summary: marquee with display:inline causes crash
Product: WebKit Reporter: Shinichiro Hamaji <hamaji>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, hyatt, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch v1
none
Patch v2
none
Patch v3
none
Patch v4 mitz: review+

Shinichiro Hamaji
Reported 2009-12-10 04:44:34 PST
The following HTML makes WebKit crash. <body> <div style="opacity: 0.9;"> <marquee style="opacity: 0.9; display: inline;">No crash means PASS</marquee> </div> </body> This happens because the marquee code assumes RenderLayer's renderer is a box.
Attachments
Patch v1 (5.93 KB, patch)
2009-12-10 04:46 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
Patch v2 (6.00 KB, patch)
2009-12-11 17:43 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
Patch v3 (5.89 KB, patch)
2009-12-13 23:52 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
Patch v4 (5.25 KB, patch)
2009-12-17 02:39 PST, Shinichiro Hamaji 		mitz: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Shinichiro Hamaji
Comment 1 2009-12-10 04:46:30 PST
Created attachment 44606 [details] Patch v1
WebKit Review Bot
Comment 2 2009-12-10 04:50:19 PST
style-queue ran check-webkit-style on attachment 44606 [details] without any errors.
Eric Seidel (no email)
Comment 3 2009-12-11 15:51:42 PST
Comment on attachment 44606 [details] Patch v1 renderMarquee() could be const, no? I expect there is lots of code which uses layer()->renderBox(). I wonder how much of the rest of it is wrong.
Shinichiro Hamaji
Comment 4 2009-12-11 17:43:29 PST
Created attachment 44719 [details] Patch v2
WebKit Review Bot
Comment 5 2009-12-11 17:44:10 PST
style-queue ran check-webkit-style on attachment 44719 [details] without any errors.
Shinichiro Hamaji
Comment 6 2009-12-11 17:49:55 PST
> renderMarquee() could be const, no? Fixed, thanks. > I expect there is lots of code which uses layer()->renderBox(). I wonder how > much of the rest of it is wrong. Yeah, I think there would be some other crashes, but I couldn't find other crashes so far. I'm guessing toRenderBox in RenderLayer::calculate{,Clip}Rects can cause crashes, but I'm not sure.
Shinichiro Hamaji
Comment 7 2009-12-13 23:52:04 PST
Created attachment 44777 [details] Patch v3
Shinichiro Hamaji
Comment 8 2009-12-13 23:52:41 PST
Fixed a wrong changelog diff.
WebKit Review Bot
Comment 9 2009-12-13 23:55:57 PST
style-queue ran check-webkit-style on attachment 44777 [details] without any errors.
mitz
Comment 10 2009-12-15 09:07:20 PST
Comment on attachment 44777 [details] Patch v3 I don’t think this is correct. You should just not create a RenderMarquee for an inline. Just change the condition in RenderLayer::styleChanged() to also check that the renderer is a box.
Shinichiro Hamaji
Comment 11 2009-12-17 02:39:52 PST
Created attachment 45047 [details] Patch v4
WebKit Review Bot
Comment 12 2009-12-17 02:40:10 PST
style-queue ran check-webkit-style on attachment 45047 [details] without any errors.
Shinichiro Hamaji
Comment 13 2009-12-17 02:47:07 PST
Ah, I see. I was unsure if marquee will never be inline. Thanks! I think we still need the fix for HTMLMarqueeElement as the code assumes it has box renderer.
mitz
Comment 14 2009-12-17 07:37:07 PST
Comment on attachment 45047 [details] Patch v4 > +RenderMarquee* HTMLMarqueeElement::renderMarquee() const > +{ > + if (renderer() && renderer()->hasLayer() && renderBoxModelObject()->layer()->marquee()) > + return renderBoxModelObject()->layer()->marquee(); > + return 0; You don’t need the “&& renderBoxModelObject()->layer()->marquee()” condition.
Shinichiro Hamaji
Comment 15 2009-12-17 21:42:41 PST
Committed r52299: <http://trac.webkit.org/changeset/52299>
Note You need to log in before you can comment on or make changes to this bug.