Bug 91003

Summary:

ASSERT(genChild->isListMarker() || genChild->style()->styleType() == FIRST_LETTER) triggered on flex-box content

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Layout and Rendering

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, inferno, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Reproduction: should hit the ASSERT in Debug	none
Proposed change 1: Do not return the flex box wrapper but search for the proper generated content parent.	none
Patch for landing	none

Julien Chaffraix

Reported 2012-07-11 13:00:14 PDT

The ASSERT can be hit pretty easily when a flex box (deprecated or not) is involved (see test case). It seems that the code is confused in RenderObjectChildList::updateBeforeAfterStyle. My analysis is that the parent |beforeAfterParent| is wrongly chosen which makes us not update the right children.

Attachments
Reproduction: should hit the ASSERT in Debug (157 bytes, text/html) 2012-07-11 13:00 PDT, Julien Chaffraix	no flags	Details
Proposed change 1: Do not return the flex box wrapper but search for the proper generated content parent. (5.81 KB, patch) 2012-07-11 13:14 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Patch for landing (5.96 KB, patch) 2012-07-12 10:27 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Julien Chaffraix

Comment 1 2012-07-11 13:00:44 PDT

Created attachment 151755 [details] Reproduction: should hit the ASSERT in Debug

Julien Chaffraix

Comment 2 2012-07-11 13:14:06 PDT

Created attachment 151757 [details] Proposed change 1: Do not return the flex box wrapper but search for the proper generated content parent.

Abhishek Arya

Comment 3 2012-07-11 17:34:45 PDT

Comment on attachment 151757 [details] Proposed change 1: Do not return the flex box wrapper but search for the proper generated content parent. View in context: https://bugs.webkit.org/attachment.cgi?id=151757&action=review > Source/WebCore/rendering/RenderObjectChildList.cpp:292 > // Only table parts need to search for the :before or :after parent Please fix this comment since we now include flexboxes. Probably add a FIXME that we should instead use isAnonymous to cover any anonymous containers. > LayoutTests/fast/flexbox/assert-generated-deprecated-flexbox.html:5 > + content: 'Generated content wrapped in a flex-box.'; nit: this should be indented right. same for the other test.

Julien Chaffraix

Comment 4 2012-07-12 10:27:28 PDT

Created attachment 151990 [details] Patch for landing

Julien Chaffraix

Comment 5 2012-07-12 10:28:34 PDT

Comment on attachment 151757 [details] Proposed change 1: Do not return the flex box wrapper but search for the proper generated content parent. View in context: https://bugs.webkit.org/attachment.cgi?id=151757&action=review >> Source/WebCore/rendering/RenderObjectChildList.cpp:292 >> // Only table parts need to search for the :before or :after parent > > Please fix this comment since we now include flexboxes. Probably add a FIXME that we should instead use isAnonymous to cover any anonymous containers. Done, also added a FIXME that we could probably get away without the check for future reference. >> LayoutTests/fast/flexbox/assert-generated-deprecated-flexbox.html:5 >> + content: 'Generated content wrapped in a flex-box.'; > > nit: this should be indented right. same for the other test. Done.

WebKit Review Bot

Comment 6 2012-07-12 13:54:17 PDT

Comment on attachment 151990 [details] Patch for landing Clearing flags on attachment: 151990 Committed r122502: <http://trac.webkit.org/changeset/122502>

WebKit Review Bot

Comment 7 2012-07-12 13:54:29 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.