Summary: | Crash in RenderImageGeneratedContent::imagePtr() using css content: with full page zoom | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Lilek <dev+webkit> | ||||||
Component: | Layout and Rendering | Assignee: | Dave Hyatt <hyatt> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | hyatt, jchaffraix | ||||||
Priority: | P1 | Keywords: | HasReduction, InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
URL: | data:text/html,<img style="content: url(http://webkit.org/images/icon-gold.png)"> | ||||||||
Attachments: |
|
Description
Matt Lilek
2008-04-21 21:02:20 PDT
Created attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent
Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent
None of the other accessors NULL-check, why should this one? OR why shouldn't they all?
Also, if there is no way to force DRT to use full page zoom (maybe it already does? I'm not sure what zoom:200% does in webkit, if anything), then this should be a manual-test.
Ideally hyatt should comment on why RenderImageGeneratedContent doesn't null-check...
Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent
The URL in this bug does not crash for me. Which OS and build type (debug vs. release) are you experiencing this crash on?
(In reply to comment #4) > (From update of attachment 22418 [details] [edit]) > The URL in this bug does not crash for me. Which OS and build type (debug vs. > release) are you experiencing this crash on? > I have tried with a nightly and ToT/debug. You have to change the zoom factor if you want the page to crash. You may also set the option to activate the full page zoom. Taking a closer look at the code, it seems that RenderImage::intrisicSizeChanged() is called before the m_styleImage is set in RenderImageGeneratedContent which lead to the crash. The null check I have added works because it is the only method that uses m_styleImage called when executing RenderImage::intrisicSizeChanged(), which means the current patch is not solving the core of the issue. A solution could be to override intrisicSizeChanged() in RenderImageGeneratedContent to do the null check. Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent
Setting this explicitly for hyatt to review.
Comment on attachment 22418 [details]
Proposed fix: add null check in RenderImageGeneratedContent
Not correct. Right fix is not to call intrinsicStyleChanged when you don't have two non-null styles.
Patch coming.
Created attachment 22468 [details]
Patch to fix problem.
Comment on attachment 22468 [details]
Patch to fix problem.
is a layout test possible?
Fixed in r35327. |