Bug 112793 - Modify checkLayout to receive the log container as an optional parameter
Summary: Modify checkLayout to receive the log container as an optional parameter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andrei Bucur
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-20 05:51 PDT by Andrei Bucur
Modified: 2013-05-14 05:51 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2013-03-20 06:06 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2013-03-20 09:01 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Test case using a complex checkLayout() (3.25 KB, text/html)
2013-03-20 09:44 PDT, Andrei Bucur
no flags Details
Patch for landing (1.89 KB, patch)
2013-05-13 23:41 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff
Patch for landing w/ reviewer (1.88 KB, patch)
2013-05-13 23:43 PDT, Andrei Bucur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Bucur 2013-03-20 05:51:08 PDT
checkLayout in check-layout.js automatically selects the logging container based on the source node (the node or its parent). In some cases this is undesirable because it may modify the elements getting measured (if they are nested).
Comment 1 Andrei Bucur 2013-03-20 06:06:20 PDT
Created attachment 194030 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-20 07:35:27 PDT
Comment on attachment 194030 [details]
Patch

Attachment 194030 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17195388

New failing tests:
css3/flexbox/box-sizing.html
css3/flexbox/multiline-justify-content.html
css3/flexbox/flex-flow-margins.html
fast/block/margin-collapse/self-collapsing-block-with-float-children.html
http/tests/cache/subresource-failover-to-network.html
css3/flexbox/multiline-align-content-horizontal-column.html
css3/flexbox/flex-align.html
css3/flexbox/preferred-widths-orthogonal.html
css3/flexbox/preferred-widths.html
css3/flexbox/line-wrapping.html
css3/flexbox/flex-align-vertical-writing-mode.html
css3/flexbox/intrinsic-min-width-applies-with-fixed-width.html
css3/flexbox/flex-item-min-size.html
fast/css-grid-layout/auto-content-resolution-columns.html
css3/flexbox/flex-flow-margins-auto-size.html
css3/flexbox/position-absolute-child.html
css3/flexbox/flex-flow-auto-margins.html
css3/flexbox/flex-flow-padding.html
css3/flexbox/multiline.html
css3/flexbox/multiline-align-self.html
css3/flexbox/flex-align-end.html
css3/flexbox/flex-flow-orientations.html
css3/flexbox/position-absolute-children.html
css3/flexbox/flex-flow-overflow.html
css3/flexbox/flexitem.html
css3/flexbox/nested-stretch.html
css3/flexbox/flex-flow-auto-margins-no-available-space.html
css3/flexbox/true-centering.html
css3/flexbox/flex-flow-border.html
fast/block/min-max-height-percent-height-child.html
Comment 3 Andrei Bucur 2013-03-20 09:01:08 PDT
Created attachment 194071 [details]
Patch
Comment 4 Andrei Bucur 2013-03-20 09:44:24 PDT
Created attachment 194080 [details]
Test case using a complex checkLayout()
Comment 5 Darin Adler 2013-05-13 18:25:51 PDT
Comment on attachment 194071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194071&action=review

> LayoutTests/resources/check-layout.js:187
> +        var container = overrideContainer;
> +        if (!overrideContainer)
> +            container = node.parentNode.className == 'container' ? node.parentNode : node;

This can also be written:

    var container = overrideContainer || (node.parentNode.className == 'container' ? node.parentNode : node);
Comment 6 Darin Adler 2013-05-13 18:26:11 PDT
Comment on attachment 194071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194071&action=review

> LayoutTests/ChangeLog:12
> +        (.):

Seriously, don’t leave lines like this in ChangeLog unless you want me to cry.
Comment 7 Andrei Bucur 2013-05-13 23:11:18 PDT
Comment on attachment 194071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194071&action=review

>> LayoutTests/ChangeLog:12
>> +        (.):
> 
> Seriously, don’t leave lines like this in ChangeLog unless you want me to cry.

Hold your tears :). I've started removing them once people told me to do it. The patch is just a bit older than that.
Comment 8 Andrei Bucur 2013-05-13 23:11:52 PDT
Comment on attachment 194071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194071&action=review

>>> LayoutTests/ChangeLog:12
>>> +        (.):
>> 
>> Seriously, don’t leave lines like this in ChangeLog unless you want me to cry.
> 
> Hold your tears :). I've started removing them once people told me to do it. The patch is just a bit older than that.

Oh, and thanks for the review!
Comment 9 Andrei Bucur 2013-05-13 23:41:25 PDT
Created attachment 201681 [details]
Patch for landing
Comment 10 Andrei Bucur 2013-05-13 23:43:47 PDT
Created attachment 201682 [details]
Patch for landing w/ reviewer
Comment 11 WebKit Commit Bot 2013-05-14 05:51:25 PDT
Comment on attachment 201682 [details]
Patch for landing w/ reviewer

Clearing flags on attachment: 201682

Committed r150067: <http://trac.webkit.org/changeset/150067>
Comment 12 WebKit Commit Bot 2013-05-14 05:51:28 PDT
All reviewed patches have been landed.  Closing bug.