Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle break-inside: avoid on tr tags #1547

Closed
tomaszn opened this issue Jan 20, 2022 · 13 comments
Closed

Handle break-inside: avoid on tr tags #1547

tomaszn opened this issue Jan 20, 2022 · 13 comments
Labels
feature New feature that should be supported
Milestone

Comments

@tomaszn
Copy link

tomaszn commented Jan 20, 2022

Since commit 222677d table cells are split between pages, and break-inside: avoid is ignored.

@grewn0uille
Copy link
Member

Hello!
It’s now normal that WeasyPrint is able to split table cells, please see #36.
But it’s strange that break-inside: avoid is ignored. Can you please share the related HTML/CSS?

@tomaszn
Copy link
Author

tomaszn commented Jan 21, 2022

I made a reduced test case, but weasyprint CLI command hangs on it (with 100% CPU usage).

https://codepen.io/3yy/pen/BawgMGK

liZe added a commit that referenced this issue Jan 25, 2022
@grewn0uille grewn0uille added this to the 54.1 milestone Jan 25, 2022
@liZe
Copy link
Member

liZe commented Jan 25, 2022

I made a reduced test case, but weasyprint CLI command hangs on it (with 100% CPU usage).

This test case actually shows that break-inside: avoid is supported 😄. The problem is that it shouldn’t cause an infinite loop, but it should break the cell instead. That’s what happens when a block is higher than the page height, the specification allows to break the block. This is now fixed in the 54.x branch.

Going back to your issue, here is an example showing that break-inside: avoid works (at least in this example):

<style>
  @page { margin: 0; size: 3.5cm }
  body { margin: 0 }
  td { line-height: 1cm; orphans: 1; widows: 1 }
</style>

<table>
  <tr>
    <td>a</td>
  </tr>
  <tr>
    <td style="break-inside: avoid">b<br>c<br>d</td>
  </tr>
</table>

<table>
  <tr>
    <td>a</td>
  </tr>
  <tr>
    <td>b<br>c<br>d</td>
  </tr>
</table>

In the first table, break-inside: avoid avoids the page break inside the second cell, and puts it on the second page. In the second table, the cell is broken as expected.

@nicolas-chaulet
Copy link

Hey! Just stumbled across it and yes with one cell per row you are correct, but when there are multiple cells per row then the row gets split, is this to be expected?

<style>
  @page {
    margin: 0;
    size: 3.5cm;
  }
  body {
    margin: 0;
  }
  td {
    line-height: 1cm;
    orphans: 1;
    widows: 1;
  }
</style>

<table>
  <tr>
    <td>cc</td>
    <td>a</td>
  </tr>
  <tr style="break-inside: avoid">
    <td>cc</td>
    <td style="break-inside: avoid">b<br />c<br />d</td>
  </tr>
</table>

<table>
  <tr>
    <td>a</td>
  </tr>
  <tr>
    <td>b<br />c<br />d</td>
  </tr>
</table>

@liZe
Copy link
Member

liZe commented Jan 25, 2022

Hey! Just stumbled across it and yes with one cell per row you are correct, but when there are multiple cells per row then the row gets split, is this to be expected?

Hmmm… It’s probably because break-inside: avoid is not supported on tr tags. But break-inside: avoid seems to work correctly on td, don’t you think?

@nicolas-chaulet
Copy link

Yeah td are totally fine, I could not find resources as to the fact that they are not supported on tr tags though

@liZe
Copy link
Member

liZe commented Jan 25, 2022

Yeah td are totally fine, I could not find resources as to the fact that they are not supported on tr tags though

Then that’s the bug we’ll fix!

@liZe liZe changed the title Table cells are split between pages (regression?) Handle break-inside: avoid on tr tags Jan 25, 2022
@nicolas-chaulet
Copy link

Ha! Amazing!

liZe added a commit that referenced this issue Jan 25, 2022
This solution is not perfect, but at least it works for table cells (whose
top margins never collapse) and avoids the related workaround.

Related to #1547.
@liZe liZe closed this as completed in 563ee6f Jan 25, 2022
@liZe
Copy link
Member

liZe commented Jan 25, 2022

Thanks for the bug report! The different commits listed here:

  • avoid the infinite loop for big unbreakable cells,
  • improve the margin-break management, enough to remove a workaround for table cells’ first child top margin,
  • handle page-break: avoid for table rows.

These fixes are available in the master and 54.x branches, and will be included in 54.1. Real-life tests and feedback are welcome!

@tomaszn
Copy link
Author

tomaszn commented Jan 25, 2022

How is rowspan on table cells honored? Because probably all cells in a row should start together:

rowspans

    <style>
        @page {
            height: 6cm;
        }
        td {
            break-inside: avoid;
            border: red 3px solid;
        }
    </style>
    <div>
        <table>
            <tr>
                <td rowspan="2">
                    a<br>
                    a<br>
                    a<br>
                    a<br>
                    a<br>
                </td>
                <td>
                    X<br>
                    X<br>
                    X<br>
                </td>
            </tr>
            <tr>
                <td>
                    Y<br>
                    Y<br>
                    Y<br>
                </td>
            </tr>

@liZe
Copy link
Member

liZe commented Jan 25, 2022

How is rowspan on table cells honored?

Well, I suppose that nobody on Earth (or on another planet) knows that.

Tables are not fully specified. rowspan and colspan attributes can give different renderings on different web browsers. Parallel table cells fragmentation is not tested by the W3C. Some browsers even don’t implement "simple" page break properties correctly in tables (🎁 a 20-year-old Firefox bug 🎁).

So… 😀

For now, we’ll assume that fragmentation of break-inside: avoid-cells with a rowspan attribute is undefined. As long as the content is displayed, let’s say that it’s OK. If you have real use cases that can’t be solved with the current implementation, you can open a new issue with a sample, but we can’t promise that we’ll solve it soon.

@tomaszn
Copy link
Author

tomaszn commented Jan 25, 2022

Your knowledge about these technologies is astonishing!

Chromium doesn't support this specific combination either. My use case is on screenshots in the bug description, which I guess is not that uncommon in reports (pivot tables etc.).

I started using WeasyPrint because it was the only engine that didn't split the rowspanned cells :-)

@liZe
Copy link
Member

liZe commented Jan 26, 2022

I started using WeasyPrint because it was the only engine that didn't split the rowspanned cells :-)

🤣

If you can change the HTML file, you can put rows with rowspan cells in tbody tags and set break-inside: avoid on these tbody tags.

@liZe liZe added the feature New feature that should be supported label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

No branches or pull requests

4 participants