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

page-break-inside: avoid -> specific column widths of table are ignored #2395

Closed
gmeister2 opened this issue Mar 3, 2021 · 7 comments · Fixed by #2502
Closed

page-break-inside: avoid -> specific column widths of table are ignored #2395

gmeister2 opened this issue Mar 3, 2021 · 7 comments · Fixed by #2502

Comments

@gmeister2
Copy link

In version 1.0.2 the specific column widths of a table are ignored (see example below), when page-break-inside: avoid is triggered (i.e. when the table would be split between 2 pages if page-break-inside: avoid was omitted). The result is that the columns are evenly divided.

In version 0.8.5 the specific column widths are retained.

PHP 7.4 is used here.

<div class="page-break-inside-avoid">
    <h2>title</h2>

    <table class="table">
        <tbody>
        <tr>
            <td class="col-2">text</td>
            <td class="col-2">text</td>
            <td class="col-4">text</td>
            <td class="col-4">text</td>
        </tr>
        <tr>
            <td>text</td>
            <td>text</td>
	    <td>text</td>
	    <td>text</td>
        </tr>
	<tr>
            <td>text</td>
            <td>text</td>
	    <td>text</td>
	    <td>text</td>
        </tr>
        </tbody>
    </table>
</div>
.page-break-inside-avoid {
    page-break-inside: avoid;
}

.table {
    display: table;
    table-layout: fixed;
    width: 100%;
    max-width: 100%;
    margin-bottom: 30px;
    border-collapse: separate;
    border-spacing: 2px;
}

.col-2 {
    width: 16.66%;
}

.col-4 {
    width: 33.33%;
}
@gmeister2
Copy link
Author

Possibly related to:
#2353

@bsweeney
Copy link
Member

bsweeney commented May 5, 2021

Do you have a full example? This is more likely related to issue #1432, but that's assuming that the same was seen in 0.8.5, which you indicate was not the case.

@bsweeney
Copy link
Member

bsweeney commented May 5, 2021

I can confirm, however, that fixed table layout does work on the first page. So this may be related but perhaps the issue is a bit more complex than I initially thought.

@bsweeney bsweeney added this to the 1.0.3 milestone May 5, 2021
@bsweeney
Copy link
Member

bsweeney commented May 5, 2021

OK, it does look like fixed table layout may not be sticky across pages, though I think the issue is a little more complex. A simple table works OK for me in initial testing.

@gmeister2
Copy link
Author

Thanks for your answers.

I made an example, with which you can test the issue (version 1.0.2).

Table 2 is where the issue occurs. Table 1 only serves to trigger it, this with the help of the variable $count, for example:

  • if $count = 25 -> specific widths of Table 2 are ok
  • if $count = 35 -> specific widths of Table 2 are not ok (columns are evenly divided)
  • if $count = 45 -> specific widths of Table 2 are ok

I also double checked with version 0.8.5, there all 3 above scenarios work ok.

public function generateHtml(): string
    {
        ob_start();?>
        <html>
        <head>
            <style>
                .page-break-inside-avoid {
                    page-break-inside: avoid;
                }

                .table {
                    display: table;
                    table-layout: fixed;
                    width: 100%;
                    max-width: 100%;
                    margin-bottom: 30px;
                    border-collapse: separate;
                    border-spacing: 2px;
                }

                .col-2 {
                    width: 16.66%;
                }

                .col-4 {
                    width: 33.33%;
                }

                .grey {
                    background-color: grey;
                }

                .orange {
                    background-color: orange;
                }

                .green {
                    background-color: green;
                }
            </style>
        </head>
        <body>

        <h2>Table 1</h2>

        <?php $count = 35; ?>

        <table class="table">
            <tbody>
            <?php for ($i = 0; $i < $count; $i++): ?>
                <tr>
                    <td class="col-4 grey">col 4</td>
                    <td class="col-4 grey">col 4</td>
                    <td class="col-4 grey">col 4</td>
                </tr>
            <?php endfor; ?>
            </tbody>
        </table>

        <div class="page-break-inside-avoid">
            <h2>Table 2</h2>

            <table class="table">
                <tbody>
                <tr>
                    <td class="col-2 orange">col 2</td>
                    <td class="col-2 orange">col 2</td>
                    <td class="col-4 green">col 4</td>
                    <td class="col-4 green">col 4</td>
                </tr>
                <tr>
                    <td class="orange">col 2</td>
                    <td class="orange">col 2</td>
                    <td class="green">col 4</td>
                    <td class="green">col 4</td>
                </tr>
                <tr>
                    <td class="orange">col 2</td>
                    <td class="orange">col 2</td>
                    <td class="green">col 4</td>
                    <td class="green">col 4</td>
                </tr>
                <tr>
                    <td class="orange">col 2</td>
                    <td class="orange">col 2</td>
                    <td class="green">col 4</td>
                    <td class="green">col 4</td>
                </tr>
                <tr>
                    <td class="orange">col 2</td>
                    <td class="orange">col 2</td>
                    <td class="green">col 4</td>
                    <td class="green">col 4</td>
                </tr>
                </tbody>
            </table>
        </div>

        </body>
        </html>
        <?php
        $html = ob_get_clean();

        return trim($html);
    }

Mellthas added a commit to Mellthas/dompdf that referenced this issue Jul 31, 2021
Node attributes are always string, so when a table was laid out, and
then a page break occured before the table at a later point, `$colspan`
and `$rowspan` would always be string values and fail the checks below,
most notably the check where the minimum column width is set, so that
all columns had a `0` minimum width.

Fixes dompdf#2395
Mellthas added a commit to Mellthas/dompdf that referenced this issue Jul 31, 2021
Node attributes are always string, so when a table was laid out, and
then a page break occurred before the table at a later point, `$colspan`
and `$rowspan` would always be string values and fail the checks below,
most notably the check where the minimum column width is set, so that
all columns received `0` minimum width.

Fixes dompdf#2395
@Mellthas
Copy link
Member

Fixed by #2502.

@uncaught
Copy link

I've run into the same issue and can confirm that #2502 fixes it! Can this get merged/released?

@bsweeney bsweeney modified the milestones: 1.1.2, 1.1.0 Sep 8, 2021
bsweeney pushed a commit that referenced this issue Sep 9, 2021
Node attributes are always string, so when a table was laid out, and
then a page break occurred before the table at a later point, `$colspan`
and `$rowspan` would always be string values and fail the checks below,
most notably the check where the minimum column width is set, so that
all columns received `0` minimum width.

Fixes #2395
@bsweeney bsweeney linked a pull request Sep 9, 2021 that will close this issue
@bsweeney bsweeney closed this as completed Sep 9, 2021
liam-milbel added a commit to Milestone-Belanova/dompdf that referenced this issue Sep 16, 2021
* Moving the PDF version to a `const`

* Remove unused import

* Coding standards: Enable more PSR-2 rules

Enable rules which should be uncontroversial and easy to fix.

* Apply coding-standards changes to files

* Travis: Also check tests dir for coding standards

* Update IMagick version check logic

fixes dompdf#1296

* Fix scoping issue for IMagick version check

* fix Trying to access array offset on value of type null (dompdf#2227)

* fix Trying to access array offset on value of type null

* space

* fix css selector issue

* Fall back to decimals for roman numerals, if out of range

Consistent with how web browsers do it.

See https://en.wikipedia.org/wiki/Roman_numerals#Standard_form

* Stylesheet: Rename and document `_image` method for clarity

* Stylesheet: Make `resolve_url` public to reduce code duplication

* FontMetrics: Fix text-width calculations when using different backends

Fixes issue dompdf#2403.

* FontMetrics: Use direct access for protected properties

* Extend tests for resolving URLs in stylesheets

Without `$sheet->set_base_path(__DIR__)`, the stylesheet location would
be the location of phpunit.

* Fix instantiating CPDF Adapter without Dompdf

* Avoid offset error in CPDF line trasparency logic

related to dompdf#2227

* Use strict comparison in CPDF transparency logic

* Stylesheet: Improve return-value check

* Remove temporary file created when caching an image. (dompdf#2411)

* Remove temporary file created when caching an image.
dompdf#2377

* move unlink() to exception catch block

* Fix text-width calulation with letter spacing (dompdf#2414)

* Consistent doc for `get_text_width` arguments

To clear up the confusion about whether word and char spacing can be
float.

* CPDF: Fix text-width calulation with letter spacing

The letter spacing was counted twice for every space character.

* PDFLib: Fix text-width calculation with letter spacing

See discussion in dompdf#2414.

* Use OS-indepenent file path for unit test

* BlockFrameReflower: Always resolve `auto` widths

* Fix undefined variable (dompdf#2466)

* Bump version to 0.8.6

* Reset version string to commit hash

* Bump version to 1.0.1

* Reset version string to commit hash

* Bump version to 1.0.2

* Reset version string to commit hash

* Fix undefined variable

* Fix indentation

* Fix undefined variable

* Update src/FontMetrics.php

* Update src/FontMetrics.php

Co-authored-by: Brian Sweeney <brian@eclecticgeek.com>

* Add support for `page-break-before/-after` on table rows

pr dompdf#2512:

* Fix some typos

* Update documentation text for `_page_break_allowed`

Mainly to clear up a few unclear points that have changed. The comment
about table headers does not apply anymore.

* Add support for `page-break-before/-after` on table rows

Caveat: `page-break-after: always` does not work if the row is the last
of its table row group. I want to keep things simple now; this might be
better fixed in a proper rework of the page-break-properties handling,
e.g. by properly propagating the property values as per
https://www.w3.org/TR/css-break-3/#break-propagation.

Fixes dompdf#329

* BlockFrameReflower: Fixes to height calculations (dompdf#2511)

Brings the code closer to the spec:
* Always resolve `auto` heights
* Apply `min-/max-height` independently of the `overflow` property
value
* `min-height` overrules `max-height` (swapping removed)

* Prevent case exception in text renderer (dompdf#2423)

In case of " auto " width is throws an error as following and breaks the PDF render.
ErrorException: A non-numeric value encountered in /Path_To_Project/vendor/dompdf/dompdf/src/Renderer/Text.php:158

type casting the $width to float will prevent the ErrorException

* Resolve auto margins in more cases (dompdf#2531)

Fixes dompdf#1482

* Properly skip checking for page breaks on empty text nodes (dompdf#2506)

Before, page breaks before empty text nodes were not allowed, but they
were still checked; and as they report their line height as the space
needed, they could force a page break, which then would continue to
search backwards in the tree for an element that allowed a page break
(even though it would fit on the current page).

Based on work in dompdf#1356
Fixes dompdf#2468

* Fix table column widths after page break (dompdf#2502)

Node attributes are always string, so when a table was laid out, and
then a page break occurred before the table at a later point, `$colspan`
and `$rowspan` would always be string values and fail the checks below,
most notably the check where the minimum column width is set, so that
all columns received `0` minimum width.

Fixes dompdf#2395

* Add support for any inline-positioned frames in text alignment

This notably adds support for inline-block.

Fixes dompdf#1761

* Fix `justify` text alignment with letter spacing

The old logic seemed to correct for wrong text-width calculation
involving letter spacing when using the CPDF backend. With the fix in
dompdf#2414 this is no longer needed.

* Improve support for relative positioning

* Move handling of relative positioning to after the reflow process, so
that it does not affect layout.
* Handle `right` and `bottom` properties plus `auto` values (except for
RTL language support, which is an easy FIXME).
* As a side-effet, this adds support for relative positioning on inline
and inline-block elements.

Fixes dompdf#516

* Fix logic error in `add_frame_to_line` function

The block must check its own content-box width, not the content-box
width of its containing block when placing child frames.

Fixes dompdf#2476.

* Apply line shift to justified lines with break

* Tweak Style border/outline method color handling

fixes dompdf#2367

* BlockFrameReflower: Fix handling of `100%` width

The condition is wrong, as an `auto` width can resolve to more than
100% if the minimum width of the content is larger.

* BlockFrameReflower: Remove swapping of `min-/max-width`

Per spec, `min-width` overrules `max-width`.

https://www.w3.org/TR/CSS21/visudet.html#min-max-widths

* Remove erroneous ‘degenerate’ case handling in `get_min_max_width`

If the frame has no children, it might still have a fixed width set.

Fixes dompdf#2366

* Small code cleanups (dompdf#2547)

* Clean up `get_margin_height/width` functions

For simplicity, return a `float` value consistently.
The `get_break_margins` method was unused.

* TextReflower: Small cleanup for `get_min_max_width`

Also, add type declarations to `_collapse_white_space`.

* Clean up `split` method signatures

Consistent argument names and more type declarations.

Co-authored-by: Thomas Landauer <thomas@landauer.at>
Co-authored-by: Till Berger <till@mellthas.de>
Co-authored-by: Brian Sweeney <brian@eclecticgeek.com>
Co-authored-by: Edwin Manuel Cerrón Angeles <xerron.angels@gmail.com>
Co-authored-by: Ion Bazan <ion.bazan@gmail.com>
Co-authored-by: bradbulger <1234634+bradbulger@users.noreply.github.com>
Co-authored-by: Jáchym Toušek <enumag@gmail.com>
Co-authored-by: Madi <mudasser.ismail@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants