Bug 13558 - REGRESSION: Crash when loading document with absolutely positioned generated content on inline element
Summary: REGRESSION: Crash when loading document with absolutely positioned generated ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Dave Hyatt
URL:
Keywords: HasReduction, Regression
Depends on:
Blocks:
 
Reported: 2007-05-01 00:01 PDT by Elliott Sprehn
Modified: 2007-05-01 15:52 PDT (History)
1 user (show)

See Also:


Attachments
Webkit Crash Log (22.21 KB, text/plain)
2007-05-01 00:01 PDT, Elliott Sprehn
no flags Details
Crash Test Case (423 bytes, text/html)
2007-05-01 00:04 PDT, Elliott Sprehn
no flags Details
Patch to allow positioned/floating generated content inside an inline. (871 bytes, patch)
2007-05-01 15:40 PDT, Dave Hyatt
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2007-05-01 00:01:13 PDT
When loading a document with an inline element that has absolutely positioned generated content Webkit crashes.

This is a new bug and does not occur in Safari 2.

Reproducible in latest Webkit Nightly (Mon Apr 30 11:37:42 GMT 2007).

Note that this CRASHES the browser IMMEDIATELY when it loads the document. Careful loading the test cases, you will lose whatever you had open.
Comment 1 Elliott Sprehn 2007-05-01 00:01:57 PDT
Created attachment 14288 [details]
Webkit Crash Log
Comment 2 Elliott Sprehn 2007-05-01 00:04:05 PDT
Created attachment 14289 [details]
Crash Test Case

This test case causes Webkit to crash immediately when loading the page.

Test case uses :after, but this happens with :before too.
Comment 3 Alexey Proskuryakov 2007-05-01 02:08:58 PDT
Confirmed with r21199.
Comment 4 mitz 2007-05-01 07:00:14 PDT
The test case generates a positioned inline. The pseudo element's initial style is display: inline; position: absolute. adjustRenderStyle fixes it, changing display to block. But then updateBeforeAfterContentForContainer has the following:

    if (isInlineFlow() && !pseudoElementStyle->isDisplayInlineType())
        // According to the CSS2 spec (the end of section 12.1), the only allowed
        // display values for the pseudo style are NONE and INLINE for inline flows.
        // FIXME: CSS2.1 lifted this restriction, but block display types will crash.
        // For now we at least relax the restriction to allow all inline types like inline-block
        // and inline-table.
        pseudoElementStyle->setDisplay(INLINE);

which resets display to inline, leading to the crash.

I don't know what crash the comment is talking about. Firefox 3 still seems to enforce the inline/none rule, but Opera allows block.

To fix this bug, you can either allow block for positioned generated content (assuming it won't trigger the crash the comment talks about), disallow positioned generated content (which would mimic Firefox, it seems), or allow block for all generated content (and fix the mysterious crash from the comment, if it still exists; that would match Opera).
Comment 5 Dave Hyatt 2007-05-01 15:07:37 PDT
Mitz, if you allow block display types (of non-positioned generated content) inside an inline, you will crash.  The flow splitting code can't deal with having to split flows across a generated content block.  The mutation of the style was a stopgap measure until those crashes could be dealt with.

Comment 6 Dave Hyatt 2007-05-01 15:09:19 PDT
This can be fixed by checking the position and float properties as well, since we can safely allow both floating and positioned content inside an inline.

Comment 7 Dave Hyatt 2007-05-01 15:40:26 PDT
Created attachment 14303 [details]
Patch to allow positioned/floating generated content inside an inline.

Will include the test case attached to the bug when I land.
Comment 8 Dave Hyatt 2007-05-01 15:42:21 PDT
Comment on attachment 14303 [details]
Patch to allow positioned/floating generated content inside an inline.

darin sez r=me
Comment 9 Dave Hyatt 2007-05-01 15:52:46 PDT
Fixed