RESOLVED FIXED 50858
Set all RenderBlocks as replaced when an inline display type is specified 
https://bugs.webkit.org/show_bug.cgi?id=50858
Summary Set all RenderBlocks as replaced when an inline display type is specified
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
DetailsFormatted DiffDiff
Patch (8.74 KB, patch)
2011-01-11 11:43 PST, James Robinson 		dglazkov: review+
DetailsFormatted DiffDiff
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.