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

[FW][FIX] base: cut multipage multi-documents on top heading #48374

Closed

Conversation

@fw-bot
Copy link
Contributor

fw-bot commented Mar 25, 2020

When we print a report for multiple records, we call wkhtmltopdf one
time and to save each document separately, we will split the PDF based
on its outline if it is available.

The outline is generated by wkhtmltopdf based on headings
(H1,H2,H3,H4,H5,H6,H7,H8,H9 elements) for example this document:

<h1>hello</h1>
<h2>world</h2>
<h1>!</h1>

has this abbreviated outline structure:

/Outlines: {
    '/First': {
        '/Title': 'hello'
        '/First': {
            '/Title': 'world'
        },
        '/Next': {
            '/Title': '!'
        }
    }
}

But the current heuristic did not take into account level of headings,
so if the document had a lower-level headings this could break the
multi-printing of these reports.

So for the example above, the document would be cut in 3 when in reality
we only want to cut it twice over the top level heading (<h1/> here).

An existing issue in Odoo is in l10n_in_sale, <h6/> element are added
to the report invoice line so instead of only one <h2/> heading
containing the invoice name per document, there was an additional
heading per invoice line which broke the heuristic.

note: we also add an assertion to ensure first heading is on first page

opw-2188767

Forward-Port-Of: #48143
Forward-Port-Of: #48099

When we print a report for multiple records, we call wkhtmltopdf one
time and to save each document separately, we will split the PDF based
on its outline if it is available.

The outline is generated by wkhtmltopdf based on headings
(H1,H2,H3,H4,H5,H6,H7,H8,H9 elements) for example this document:

```
<h1>hello</h1>
<h2>world</h2>
<h1>!</h1>
```

has this abbreviated outline structure:

```
/Outlines: {
    '/First': {
        '/Title': 'hello'
        '/First': {
            '/Title': 'world'
        },
        '/Next': {
            '/Title': '!'
        }
    }
}
```

But the current heuristic did not take into account level of headings,
so if the document had a lower-level headings this could break the
multi-printing of these reports.

So for the example above, the document would be cut in 3 when in reality
we only want to cut it twice over the top level heading (`<h1/>` here).

An existing issue in Odoo is in l10n_in_sale, `<h6/>` element are added
to the report invoice line so instead of only one `<h2/>` heading
containing the invoice name per document, there was an additional
heading per invoice line which broke the heuristic.

note: we also add an assertion to ensure first heading is on first page.

note: allowing several top-level heading on same page has been
implemented so reports that did not work in 11.0 but worked in 12.0 and
over (thanks to 573e577) still work after this change.

opw-2188767
closes #48099

X-original-commit: 63c2478
@fw-bot

This comment has been minimized.

Copy link
Contributor Author

fw-bot commented Mar 25, 2020

Ping @nle-odoo
This PR targets master and is the last of the forward-port chain containing:

To merge the full chain, say

@fw-bot r+

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added the CI 🤖 label Mar 25, 2020
@C3POdoo C3POdoo added the OE label Mar 25, 2020
Copy link
Contributor

nle-odoo left a comment

robodoo r+

@nle-odoo

This comment has been minimized.

Copy link
Contributor

nle-odoo commented Mar 26, 2020

fw-bot r+

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 26, 2020

I'm sorry, @nle-odoo. This PR is already reviewed, reviewing it again is useless.

@robodoo robodoo added the r+ 👌 label Mar 26, 2020
robodoo pushed a commit that referenced this pull request Mar 26, 2020
When we print a report for multiple records, we call wkhtmltopdf one
time and to save each document separately, we will split the PDF based
on its outline if it is available.

The outline is generated by wkhtmltopdf based on headings
(H1,H2,H3,H4,H5,H6,H7,H8,H9 elements) for example this document:

```
<h1>hello</h1>
<h2>world</h2>
<h1>!</h1>
```

has this abbreviated outline structure:

```
/Outlines: {
    '/First': {
        '/Title': 'hello'
        '/First': {
            '/Title': 'world'
        },
        '/Next': {
            '/Title': '!'
        }
    }
}
```

But the current heuristic did not take into account level of headings,
so if the document had a lower-level headings this could break the
multi-printing of these reports.

So for the example above, the document would be cut in 3 when in reality
we only want to cut it twice over the top level heading (`<h1/>` here).

An existing issue in Odoo is in l10n_in_sale, `<h6/>` element are added
to the report invoice line so instead of only one `<h2/>` heading
containing the invoice name per document, there was an additional
heading per invoice line which broke the heuristic.

note: we also add an assertion to ensure first heading is on first page.

note: allowing several top-level heading on same page has been
implemented so reports that did not work in 11.0 but worked in 12.0 and
over (thanks to 573e577) still work after this change.

opw-2188767
closes #48099

closes #48374

X-original-commit: 63c2478
Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>
@robodoo robodoo closed this Mar 26, 2020
@robodoo robodoo deployed to merge Mar 26, 2020 Active
vtu-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 30, 2020
When we print a report for multiple records, we call wkhtmltopdf one
time and to save each document separately, we will split the PDF based
on its outline if it is available.

The outline is generated by wkhtmltopdf based on headings
(H1,H2,H3,H4,H5,H6,H7,H8,H9 elements) for example this document:

```
<h1>hello</h1>
<h2>world</h2>
<h1>!</h1>
```

has this abbreviated outline structure:

```
/Outlines: {
    '/First': {
        '/Title': 'hello'
        '/First': {
            '/Title': 'world'
        },
        '/Next': {
            '/Title': '!'
        }
    }
}
```

But the current heuristic did not take into account level of headings,
so if the document had a lower-level headings this could break the
multi-printing of these reports.

So for the example above, the document would be cut in 3 when in reality
we only want to cut it twice over the top level heading (`<h1/>` here).

An existing issue in Odoo is in l10n_in_sale, `<h6/>` element are added
to the report invoice line so instead of only one `<h2/>` heading
containing the invoice name per document, there was an additional
heading per invoice line which broke the heuristic.

note: we also add an assertion to ensure first heading is on first page.

note: allowing several top-level heading on same page has been
implemented so reports that did not work in 11.0 but worked in 12.0 and
over (thanks to 573e577) still work after this change.

opw-2188767
closes odoo#48099

closes odoo#48374

X-original-commit: 63c2478
Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.