Bug 130499

Summary:

[CSS Regions] Monolithic elements should not affect the layout of the content outside its region

Product:

WebKit

Reporter:

Andrei Bucur <abucur>

Component:

Layout and Rendering

Assignee:

Radu Stavila <stavila>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, esprehn+autocc, glenn, kondapallykalyan, stavila, WebkitBugTracker, zoltan

Priority:

Keywords:

AdobeTracked

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

57312

Attachments:

Description	Flags
Patch	none
Patch v2	hyatt: review-
Patch v3	hyatt: review-
Patch which implements reviewer feedback	none

Andrei Bucur

Reported 2014-03-20 02:01:18 PDT

See http://codepen.io/abucur/pen/JtevB/

Attachments
Patch (32.54 KB, patch) 2014-04-03 06:17 PDT, Radu Stavila	no flags	Details Formatted Diff Diff
Patch v2 (32.50 KB, patch) 2014-04-03 08:50 PDT, Radu Stavila	hyatt: review-	Details Formatted Diff Diff
Patch v3 (24.02 KB, patch) 2014-04-08 09:20 PDT, Radu Stavila	hyatt: review-	Details Formatted Diff Diff
Patch which implements reviewer feedback (24.67 KB, patch) 2014-04-09 07:08 PDT, Radu Stavila	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radu Stavila

Comment 1 2014-04-03 06:17:17 PDT

Created attachment 228503 [details] Patch

Radu Stavila

Comment 2 2014-04-03 08:50:45 PDT

Created attachment 228511 [details] Patch v2

Dave Hyatt

Comment 3 2014-04-03 09:22:07 PDT

Comment on attachment 228511 [details] Patch v2 I do not agree with the approach taken in this bug and feel it's too intrusive into RenderBlock's layout (and FloatingObjects). It seems like you should be able to come up with an approach that is contained in the flow thread/region code.

Radu Stavila

Comment 4 2014-04-08 09:20:08 PDT

Created attachment 228843 [details] Patch v3

Dave Hyatt

Comment 5 2014-04-08 09:39:41 PDT

Comment on attachment 228843 [details] Patch v3 Ok, so I finally understand why you need this code. The basic issue is that regions have been improved to handle monolithic elements differently from printing/columns. When a monolithic element overflows a region, you're actually rendering the element entirely within the region it overflows. This is great, and I wish printing and multicolumn behaved this way as well. I could use a reminder of where the code lives that does this for when printing/multicolumn change to behave like this in the future. Anyway, I think we need a comment explaining that the reason this function exists is because monolithic elements do not get split across regions, and note that this is a difference from printing/multicolumn in the comment. The function will also need to be tweaked to check for flow threads/regions, etc. to make sure you don't turn this on for printing or multi-column (since right now the gap makes sense for those systems).

Radu Stavila

Comment 6 2014-04-09 07:08:39 PDT

Created attachment 228959 [details] Patch which implements reviewer feedback

Dave Hyatt

Comment 7 2014-04-09 09:36:00 PDT

Comment on attachment 228959 [details] Patch which implements reviewer feedback r=me

WebKit Commit Bot

Comment 8 2014-04-09 10:08:17 PDT

Comment on attachment 228959 [details] Patch which implements reviewer feedback Clearing flags on attachment: 228959 Committed r167018: <http://trac.webkit.org/changeset/167018>

WebKit Commit Bot

Comment 9 2014-04-09 10:08:20 PDT

All reviewed patches have been landed. Closing bug.

Zoltan Horvath

Comment 10 2014-04-10 14:18:00 PDT

The changelog says you've modified FloatingObjects.cpp, but I don't see it in the patch.

Radu Stavila

Comment 11 2014-04-10 14:20:37 PDT

Yes, it was in the first version of the patch and after changing it, it seems I forgot to update the CL. Sorry.

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