The first table row is pushed down by border-spacing.

The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
collateral fix. It's hopefully correct, since we now match Edge's behavior.

BUG=709387, 534751
Review-Url: https://codereview.chromium.org/2803383002
Cr-Commit-Position: refs/heads/master@{#469690}
Committed: https://chromium.googlesource.com/chromium/src/+/97b20197a53d323f7b2d92581d97817ea85ea0a4

[mstensho (USE GERRIT), 2017-04-07 22:20:25]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[mstensho (USE GERRIT), 2017-04-07 22:20:51]

mstensho@opera.com changed reviewers:
+ eae@chromium.org

[commit-bot: I haz the power, 2017-04-07 22:21:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-07 23:49:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 23:49:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[eae, 2017-04-08 06:49:35]

LGTM

[mstensho (USE GERRIT), 2017-04-08 18:50:01]

On 2017/04/08 06:49:35, eae wrote:
> LGTM

Thanks for the review! I need to figure out what's going on with
printing/avoid-setting-header-offset-on-header.html before I can land it,
though. That will probably have to wait until after Easter.

[mstensho (USE GERRIT), 2017-05-05 13:16:16]

Description was changed from

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

BUG=709387,534751
==========

to

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
particular unintended change (but hopefully correct, since we now match
Edge's behavior).

BUG=709387,534751
==========

[mstensho (USE GERRIT), 2017-05-05 13:21:18]

Description was changed from

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
particular unintended change (but hopefully correct, since we now match
Edge's behavior).

BUG=709387,534751
==========

to

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
collateral fix. It's hopefully correct, since we now match Edge's behavior.

BUG=709387,534751
==========

[mstensho (USE GERRIT), 2017-05-05 13:21:32]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 13:21:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mstensho (USE GERRIT), 2017-05-05 13:25:52]

mstensho@opera.com changed reviewers:
+ robhogan@gmail.com

[mstensho (USE GERRIT), 2017-05-05 13:29:17]

On 2017/04/08 18:50:01, mstensho wrote:
> On 2017/04/08 06:49:35, eae wrote:
> > LGTM
> 
> Thanks for the review! I need to figure out what's going on with
> printing/avoid-setting-header-offset-on-header.html before I can land it,
> though. That will probably have to wait until after Easter.

It turned out that bug 716442 was partially responsible for the failure in
printing/avoid-setting-header-offset-on-header.html . That got fixed by
https://codereview.chromium.org/2848883002/ , but there was still one problem;
the -expected.html counterpart had started to act up. It turns out that it
expected us not to break inside a portion of border-spacing that's followed by a
table row with break-inside:avoid - but this seems unreasonable to me. So I
updated the test instead.

So... time to bring in @rhogan - could you please take a look and see if you
agree?

@eae - maybe you also want to take another look (note that there are no code
changes since last time, though).

[commit-bot: I haz the power, 2017-05-05 15:20:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 15:20:05]

Dry run: This issue passed the CQ dry run.

[eae, 2017-05-05 16:30:28]

On 2017/05/05 13:29:17, mstensho wrote:
> On 2017/04/08 18:50:01, mstensho wrote:
> > On 2017/04/08 06:49:35, eae wrote:
> > > LGTM
> > 
> > Thanks for the review! I need to figure out what's going on with
> > printing/avoid-setting-header-offset-on-header.html before I can land it,
> > though. That will probably have to wait until after Easter.
> 
> It turned out that bug 716442 was partially responsible for the failure in
> printing/avoid-setting-header-offset-on-header.html . That got fixed by
> https://codereview.chromium.org/2848883002/ , but there was still one problem;
> the -expected.html counterpart had started to act up. It turns out that it
> expected us not to break inside a portion of border-spacing that's followed by
a
> table row with break-inside:avoid - but this seems unreasonable to me. So I
> updated the test instead.
> 
> So... time to bring in @rhogan - could you please take a look and see if you
> agree?
> 
> @eae - maybe you also want to take another look (note that there are no code
> changes since last time, though).

If rhogan is happy with it then so am I.

[rhogan, 2017-05-05 17:00:44]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2017-05-05 17:00:45]

lgtm

[rhogan, 2017-05-05 17:00:45]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2803383002/#ps100001
(title: "break-inside:avoid on a row shouldn't prevent breaking inside adjacent
border spacing")

[commit-bot: I haz the power, 2017-05-05 17:01:12]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 17:09:13]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494003644439580,
"parent_rev": "a0a5a62f736a57278092edbf232f61d68394b02e", "commit_rev":
"97b20197a53d323f7b2d92581d97817ea85ea0a4"}

[commit-bot: I haz the power, 2017-05-05 17:09:34]

Description was changed from

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
collateral fix. It's hopefully correct, since we now match Edge's behavior.

BUG=709387,534751
==========

to

==========
The first table row is pushed down by border-spacing.

We need to make sure this happens *before* laying it out when inside a
fragmentation context.

Added tests fast/multicol/balance-table-with-border-spacing.html and
fragmentation/table-with-border-spacing.html for this.

This change also fixes breaking inside border-spacing adjacent to table
rows with break-inside:avoid set. There should be no reason to prevent
breaking inside border spacing, just because it's adjacent to such table
rows, but it looks like this was the behavior we got, by accident.
Updated printing/avoid-setting-header-offset-on-header.html accordingly
and threw in an additional test
fragmentation/border-spacing-break-before-unbreakable-row.html for this
collateral fix. It's hopefully correct, since we now match Edge's behavior.

BUG=709387,534751

Review-Url: https://codereview.chromium.org/2803383002
Cr-Commit-Position: refs/heads/master@{#469690}
Committed:
https://chromium.googlesource.com/chromium/src/+/97b20197a53d323f7b2d92581d97...
==========

[commit-bot: I haz the power, 2017-05-05 17:09:36]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/97b20197a53d323f7b2d92581d97...