Repository: Kozea/WeasyPrint

Issue:
Empty pages from page-break-before/after use wrong page selector
**Problem description**: Using ``page-break-before/after: right/left`` can create empty pages which is of course intended. However, these pages use the wrong css selector in my case. In particular, they use a different selector than the pages before and after.

**Example**: In the following example, I would expect all pages to be white, because all the content is within the `main` element. However, the empty page, which is created due to `page-break-before: right` is not white, but blue.

```html
<html>
<head>
    <style>
        main { page: main_page; }
        h1 { page-break-before: right; }
        @page { background-color: blue; }
        @page main_page { background-color: white; }
    </style>
</head>
<body>
    <main>
        <h1>First heading</h1>
        <h1>Second heading</h1>
    </main>
</body>
</html>
```

**Version**:  Weasyprint 51

Hints from issue comments:
@Tontyna: This is by purpose:

https://github.com/Kozea/WeasyPrint/blob/27c600f18f025d8a7bffb4e5e0f861014881198e/weasyprint/layout/pages.py#L725-L726

...and possibly according to the spec... according to the spec, a blank page is **content empty**. Contemplating this expression, we must confess, that there is no `<main>` box to match the CSS rule `main { page: main_page; }`. And the blue page, despite being a left page, is right.

BTW: According to this nitpicking the CSS selector `@page main_page:blank` should be invalid.
@timtroendle: Thanks for the update! I cannot judge whether this is according to the spec. It just doesn't feel intuitive to me as the page is before the opening and after the closing tag of `main`. But I understand that there is no content on the page and thus it's considered blank. Using `@page main_page:blank` could be a nice workaround, but as you correctly point out, it isn't.@Tontyna: I didn't find anything in the spec mentioning (or forbidding) the `:blank` pseudo selector in context of named pages. My comment was supposed to be a bit sarcastically. 
The blue page feels wrong.

In fact: @liZe, who knows the spec better than I do, added the bug label.@liZe: > In fact: @liZe, who knows the spec better than I do, added the bug label.

There’s a bug because we have to keep the current page name on empty pages created by `break` rules.@Tontyna: The (current/last used) page name is present (I debugged that), but somebody explicitly decided to remove it:

https://github.com/Kozea/WeasyPrint/blob/c443b9c9a61d82bd581c6f6aac7df746368c0073/weasyprint/layout/pages.py#L725-L726
@Tontyna: Nevertheless I'm wondering whether a blank page, happening between two different named pages should belong to the previous or to the next page's namespace? Should that depend on the break-rule -- `break-after` vs. `break-before`?@liZe: > The (current/last used) page name is present (I debugged that), but somebody explicitly decided to remove it:

Yes, this code is (probably) useful for example when we break after an element with a named page.@liZe: > Nevertheless I'm wondering whether a blank page, happening between two different named pages should belong to the previous or to the next page's namespace? Should that depend on the break-rule -- `break-after` vs. `break-before`?

I’m not sure of anything anymore.

With the previous example, shouldn’t the `main` box be displayed on the empty page? That would make sense, and then the answer would be : "it belongs to the parent namespace". I have to read the spec again.@Tontyna: > I have to read the spec again.

Good luck!@tokyovigilante: Hi, is there any way to work round this issue? I am putting together a book using numbered sections, and having the next section start on a right page means I lose control of the formatting of headers and footers on the blank pages before.@timtroendle: Hi @tokyovigilante, I do not have a good workaround, other than making sure the pages before and after the inserted blank page use the default `page` style so that the style on all three pages is the same.

In my case, I needed two different styles: one for the part before the first main section and one for the main section (in which this problem occurred). I ensured the main section uses the default `page` style and I styled all pages before the main page with explicit `@page :nth(...)` selectors. Inelegant and brittle, but worked for me.

Of course this won't work for more complicated styles.@tokyovigilante: @timtroendle Thanks for the comments. I'm trying to make my front matter section (between cover and contents page) use roman numerals for page numbers, and then my default `page` style use Section-Page numbering. 

Unfortunately the last page in the front matter section reverts to the default as my style is tied to each page keeping its section name. I can use hacks as you suggest to compensate but it is inelegant as you suggest.@liZe: This bug is now fixed and tested, the fix will be included in 65.1.

Solution patch:
diff --git a/weasyprint/layout/page.py b/weasyprint/layout/page.py
index 09165a9ae..0790d743e 100644
--- a/weasyprint/layout/page.py
+++ b/weasyprint/layout/page.py
@@ -799,6 +799,8 @@ def set_page_type_computed_styles(page_type, html, style_for):
 
 
 def _includes_resume_at(resume_at, page_group_resume_at):
+    if not page_group_resume_at:
+        return True
     (page_child_index, page_child_resume_at), = page_group_resume_at.items()
     if resume_at is None or page_child_index not in resume_at:
         return False
@@ -807,7 +809,7 @@ def _includes_resume_at(resume_at, page_group_resume_at):
     return _includes_resume_at(resume_at[page_child_index], page_child_resume_at)
 
 
-def _update_page_groups(page_groups, resume_at, next_page, root_box):
+def _update_page_groups(page_groups, resume_at, next_page, root_box, blank):
     # https://www.w3.org/TR/css-gcpm-3/#document-sequence-selectors
 
     # Remove or increment page groups.
@@ -819,35 +821,40 @@ def _update_page_groups(page_groups, resume_at, next_page, root_box):
             page_groups.pop(i - page_groups_length)
 
     # Add page groups.
-    if next_page['break'] == 'any' or not next_page['page']:
-        # We don’t have a forced page break or a named page.
-        return
-    if page_groups and page_groups[-1][0] == next_page['page']:
-        # We’re already in an element whose page name is the next page name.
-        return
+    if not blank:
+        if (resume_at and next_page['break'] == 'any') or not next_page['page']:
+            # We don’t have a forced page break or a named page.
+            return next_page['page']
+        if page_groups and page_groups[-1][0] == next_page['page']:
+            # We’re already in an element whose page name is the next page name.
+            return next_page['page']
 
     # Find the box that has the named page. It is a first in-flow child of the
     # element corresponding to resume_at.
 
     # Find element corrensponding to resume_at.
-    page_group_resume_at = copy.deepcopy(resume_at)
-    current_resume_at = page_group_resume_at
+    current_resume_at = page_group_resume_at = copy.deepcopy(resume_at) or {0: None}
     current_element = root_box
     while True:
         child_index, child_resume_at = tuple(current_resume_at.items())[-1]
+        parent_element = current_element
         current_element = current_element.children[child_index]
         if child_resume_at is None:
             break
         current_resume_at = child_resume_at
 
+    if blank:
+        # Page is blank, don’t create a new page group and return parent’s page name.
+        return parent_element.style['page']
+
     # Find the descendant with named page.
     while True:
         if current_element.style['page'] == next_page['page']:
             page_groups.append([next_page['page'], 0, page_group_resume_at])
-            return
+            return next_page['page']
         if not isinstance(current_element, boxes.ParentBox):
             # Shouldn’t happen.
-            return
+            return next_page['page']
         for i, child in enumerate(current_element.children):
             if not child.is_in_normal_flow():
                 continue
@@ -858,7 +865,7 @@ def _update_page_groups(page_groups, resume_at, next_page, root_box):
             break
         else:
             # Shouldn’t happen.
-            return
+            return next_page['page']
 
 
 def remake_page(index, page_groups, context, root_box, html):
@@ -891,9 +898,8 @@ def remake_page(index, page_groups, context, root_box, html):
         (next_page_side == 'left' and right_page) or
         (next_page_side == 'right' and not right_page) or
         (context.reported_footnotes and resume_at is None))
-    name = '' if blank else next_page['page']
     side = 'right' if right_page else 'left'
-    _update_page_groups(page_groups, resume_at, next_page, root_box)
+    name = _update_page_groups(page_groups, resume_at, next_page, root_box, blank)
     groups = tuple((name, index) for name, index, _ in page_groups)
     page_type = PageType(side, blank, name, index, groups)
     set_page_type_computed_styles(page_type, html, context.style_for)


Test patch:
diff --git a/tests/layout/test_page.py b/tests/layout/test_page.py
index 617e2358b..fa6ea4331 100644
--- a/tests/layout/test_page.py
+++ b/tests/layout/test_page.py
@@ -969,6 +969,74 @@ def test_page_groups():
     assert section.element_tag == 'section'
 
 
+@assert_no_logs
+def test_page_groups_blank_inside():
+    # Regression test for #1076.
+    pages = render_pages('''
+      <style>
+        @page { size: 100px }
+        @page div { size: 50px }
+        div { page: div }
+        p { break-before: right }
+      </style>
+      <div>
+        <p>1</p>
+        <p>2</p>
+      </div>
+    ''')
+    assert len(pages) == 3
+    for page in pages:
+        assert (page.width, page.height) == (50, 50)
+
+
+@assert_no_logs
+def test_page_groups_blank_outside():
+    pages = render_pages('''
+      <style>
+        @page { size: 100px }
+        @page p { size: 50px }
+        p { page: p; break-before: right }
+      </style>
+      <div>
+        <p>1</p>
+        <p>2</p>
+      </div>
+    ''')
+    page1, page2, page3 = pages
+    for page in (page1, page3):
+        assert (page.width, page.height) == (50, 50)
+    assert (page2.width, page2.height) == (100, 100)
+
+
+@assert_no_logs
+def test_page_groups_first_nth():
+    # Regression test for #2429.
+    pages = render_pages('''
+      <style>
+        @page { size: 100px }
+        @page div { size: 50px }
+        @page :nth(2n+1 of div) { size: 30px }
+        div { page: div; break-before: right }
+        p { break-before: page }
+      </style>
+      <div>
+        <p>1</p>
+        <p>2</p>
+        <p>3</p>
+      </div>
+      <div>
+        <p>4</p>
+        <p>5</p>
+      </div>
+    ''')
+    page1, page2, page3, page4, page5, page6 = pages
+    for page in (page1, page3, page5):
+        assert (page.width, page.height) == (30, 30)
+    for page in (page2, page6):
+        assert (page.width, page.height) == (50, 50)
+    assert (page4.width, page4.height) == (100, 100)
+
+
 @assert_no_logs
 @pytest.mark.parametrize('style, line_counts', (
     ('orphans: 2; widows: 2', [4, 3]),