Bug 133120 - [New Multicolumn] Contents in the columns following a spanner may bleed into the column preceding it
Summary: [New Multicolumn] Contents in the columns following a spanner may bleed into ...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Morten Stenshorne
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-20 06:14 PDT by Morten Stenshorne
Modified: 2022-02-10 03:54 PST (History)
9 users (show)

See Also:


Attachments
Patch (15.32 KB, patch)
2014-05-20 06:58 PDT, Morten Stenshorne
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (2.57 MB, application/zip)
2014-05-20 08:20 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (2.57 MB, application/zip)
2014-05-20 09:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (2.55 MB, application/zip)
2014-05-20 11:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (2.54 MB, application/zip)
2014-05-20 12:00 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Morten Stenshorne 2014-05-20 06:14:43 PDT
This was introduced by the fix for bug 132298 . The code removal in RenderMultiColumnSet::endFlow(). So the code there was necessary after all (it's just that we didn't have any tests for it), but it's messy, so better find a cleaner way of doing this.

How about inserting a column break at the placeholder and have a strut do the job for us?
Comment 1 Morten Stenshorne 2014-05-20 06:58:35 PDT
Created attachment 231766 [details]
Patch
Comment 2 Morten Stenshorne 2014-05-20 07:15:25 PDT
@hyatt - can you review, please?

This will require some test rebaselining, because the flow thread will become taller than before in some cases (without affecting the set heights).

I'm not exactly sure why code had to be removed from endFlow() as part of fixing bug 132298, but it was admittedly kind of ugly. I hope this new attempt won't mess up anything for compositing.
Comment 3 Build Bot 2014-05-20 08:20:45 PDT
Comment on attachment 231766 [details]
Patch

Attachment 231766 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6526303687671808

New failing tests:
fast/multicol/span/span-as-immediate-child-generated-content.html
fast/multicol/span/span-as-immediate-child-complex-splitting.html
fast/multicol/span/span-as-immediate-columns-child-dynamic.html
fast/multicol/span/span-as-immediate-columns-child.html
fast/multicol/span/span-as-nested-columns-child-dynamic.html
fast/multicol/span/span-as-nested-columns-child.html
Comment 4 Build Bot 2014-05-20 08:20:49 PDT
Created attachment 231770 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 5 Morten Stenshorne 2014-05-20 08:38:48 PDT
So, yeah, that were the "failures" I kind of expected. Need to rebaseline this. I don't know about the image failures, and how I could have caused them, though. Looks like everything is off by ~half a pixel, horizontally. Compositing issue? I only expected render tree dump differences.
Comment 6 Build Bot 2014-05-20 09:23:08 PDT
Comment on attachment 231766 [details]
Patch

Attachment 231766 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5116720486285312

New failing tests:
fast/multicol/span/span-as-immediate-child-generated-content.html
fast/multicol/span/span-as-immediate-child-complex-splitting.html
fast/multicol/span/span-as-immediate-columns-child-dynamic.html
fast/multicol/span/span-as-immediate-columns-child.html
fast/multicol/span/span-as-nested-columns-child-dynamic.html
fast/multicol/span/span-as-nested-columns-child.html
Comment 7 Build Bot 2014-05-20 09:23:12 PDT
Created attachment 231772 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2014-05-20 11:03:03 PDT
Comment on attachment 231766 [details]
Patch

Attachment 231766 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5623377108664320

New failing tests:
fast/multicol/span/span-as-immediate-child-generated-content.html
fast/multicol/span/span-as-immediate-child-complex-splitting.html
fast/multicol/span/span-as-immediate-columns-child-dynamic.html
fast/multicol/span/span-as-immediate-columns-child.html
fast/multicol/span/span-as-nested-columns-child-dynamic.html
fast/regions/cssom/region-range-for-box-crash.html
fast/multicol/span/span-as-nested-columns-child.html
Comment 9 Build Bot 2014-05-20 11:03:06 PDT
Created attachment 231778 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 Build Bot 2014-05-20 11:59:56 PDT
Comment on attachment 231766 [details]
Patch

Attachment 231766 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6674724436312064

New failing tests:
fast/multicol/span/span-as-immediate-child-generated-content.html
fast/multicol/span/span-as-immediate-child-complex-splitting.html
fast/multicol/span/span-as-immediate-columns-child-dynamic.html
fast/multicol/span/span-as-immediate-columns-child.html
fast/multicol/span/span-as-nested-columns-child-dynamic.html
fast/multicol/span/span-as-nested-columns-child.html
Comment 11 Build Bot 2014-05-20 12:00:00 PDT
Created attachment 231782 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 12 Michael Catanzaro 2016-09-17 07:16:03 PDT
Comment on attachment 231766 [details]
Patch

Hi,

Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.

To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.
Comment 13 Sam Sneddon [:gsnedders] 2022-02-10 03:54:56 PST
This is no longer applicable given Regions (which the new multicol was based on) was removed in bug 174978.