Bug 50858

Summary:

Set all RenderBlocks as replaced when an inline display type is specified

Product:

WebKit

Reporter:

James Robinson <jamesr>

Component:

New Bugs

Assignee:

James Robinson <jamesr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, dglazkov, hyatt, inferno

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
Patch	dglazkov: review+

James Robinson

Reported 2010-12-10 18:09:07 PST

Set all RenderBlocks as replaced when an inline display type is specified

Attachments
Patch (8.36 KB, patch) 2010-12-10 18:15 PST, James Robinson	no flags	Details Formatted Diff Diff
Patch (8.74 KB, patch) 2011-01-11 11:43 PST, James Robinson	dglazkov: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

James Robinson

Comment 1 2010-12-10 18:15:38 PST

Created attachment 76286 [details] Patch

Darin Adler

Comment 2 2010-12-10 18:16:31 PST

Comment on attachment 76286 [details] Patch This consolidates the code. But does it fix any still-broken cases? If so, we should add test cases for those.

James Robinson

Comment 3 2010-12-10 18:24:01 PST

I'm not entirely sure - there are some subclasses of RenderBlock that fail to set this properly now, but I'm not sure how to construct a layout test for them that produces observably bad behavior. I wasn't able to make any of them produce the same symptoms that RenderDetails had (although I only tried a few quick things). Suggestions welcome.

Abhishek Arya

Comment 4 2010-12-10 19:51:45 PST

One suggestion i have is we can have one testcase [mimicing http://trac.webkit.org/changeset/73681] that tries to set display:inline [using same class=control] on all the tags that are listed as display:block in this testfile http://trac.webkit.org/browser/trunk/WebCore/css/html.css. Dont know if this will be an acceptable approach. Also, please also remove the function call from renderdetails [added recently] :)

James Robinson

Comment 5 2011-01-10 13:23:31 PST

I'm pretty sure that this does not change behavior for any current subclasses of RenderBlock, but it may for future ones.

Dimitri Glazkov (Google)

Comment 6 2011-01-10 13:24:43 PST

James! Keep working on this patch! :P

James Robinson

Comment 7 2011-01-10 17:53:30 PST

Comment on attachment 76286 [details] Patch Requesting review - I've confirmed that this patch does not regress any existing layout tests, and as far as I can tell doesn't change behavior for any existing renderer. I believe it would only cause a behavior change if a subclass of RenderBlock was currently buggy. This should help ensure that new subclasses of RenderBlock avoid this pitfall.

Dimitri Glazkov (Google)

Comment 8 2011-01-11 09:21:05 PST

(In reply to comment #7) > (From update of attachment 76286 [details]) > Requesting review - I've confirmed that this patch does not regress any existing layout tests, and as far as I can tell doesn't change behavior for any existing renderer. I believe it would only cause a behavior change if a subclass of RenderBlock was currently buggy. This should help ensure that new subclasses of RenderBlock avoid this pitfall. Can you explain why no new tests in the ChangeLog?

James Robinson

Comment 9 2011-01-11 11:43:34 PST

Created attachment 78566 [details] Patch

Dimitri Glazkov (Google)

Comment 10 2011-01-11 12:00:57 PST

Comment on attachment 78566 [details] Patch Hooray!

James Robinson

Comment 11 2011-01-11 12:48:04 PST

Committed r75537: <http://trac.webkit.org/changeset/75537>

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