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

Images placed incorrectly when cross reference is used #808

Closed
mattieusch opened this Issue Feb 19, 2019 · 12 comments

Comments

3 participants
@mattieusch
Copy link

mattieusch commented Feb 19, 2019

There seems to be a bug in the (re)placement of images that are moved to the next page because of insufficient space on the bottom. These 'moved' images are not rendered or placed on an incorrect location. This issue only manifests itself when a (forward) cross-reference is being used before the image.

See HTML below to reproduce this on WeasyPrint 44, dev version, checked out on 2019-02-19.

<html>
<head>
    <style>
        a.ref::after {
            content: " (p. "target-counter(attr(href), page) ")";
        }
    </style>
</head>

<body class="nl">
  <p>
    <a class="ref" href="#item-1650">Link</a>
    <br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>lorem<br>
  </p>
  <img alt="" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAZAAAAGQCAIAAAAP3aGbAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4wITDwMSt6ANKwAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAD5UlEQVR42u3UMQ0AAAjAMCD4twwqOEhaCTuW0x0AH5QEgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBhiUBYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVgWACGBRgWgGEBGBZgWACGBWBYgGEBGBaAYQGGBWBYAIYFGBaAYQGGBWBYAIYFGBaAYQEYFmBYAIYFYFiAYQEYFoBhAYYFYFgAhgUYFoBhARgWYFgAhgVgWIBhARgWgGEBhgVwaQFxqQQn9wVzXwAAAABJRU5ErkJggg==">
  <p id="item-1650">Destination</p>
</body>
</html>
@liZe

This comment has been minimized.

Copy link
Member

liZe commented Feb 19, 2019

Wow. I really don't know where it comes from, I hope it's not a problem in Cairo…

@liZe liZe added the bug label Feb 19, 2019

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Feb 19, 2019

Neither do I know the reason but when fighting with re-pagination the last time I had the strange impression fact that calling make_page the second time altered (shrinked) some box-margins.

Besides being irritated an wondering what else might be affected by re-make I ignored it for the sake of the actual bug I wanted to fix. Planned to open an issue but as time goes by...

I suppose this time it's not Cairo,

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Feb 19, 2019

... ah yes, and like in @mattieusch's snippet there was a coincidence with <br>s

Edit: 😉

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Feb 19, 2019

I know what happens, but dont know where and why. Ok. With only one call to make_all_pages the 2 pages look like this:

paginated

The blue arrow points to the place where the red square image would be if it would fit on the first page. But it's too big and it's moved -- together with its red-framed div -- to the next page.

Now let's trigger repagination:

repaginated

Page 1 is forced by the cross-reffed link to be re-made. Page 2 isn't touched, aka up-to-date. But the red square image isn't inside its containing red-framed div anymore. Instead it's at the position where it would be on page 1 if it would fit on page 1; at least it's still on page 2.

@liZe -- do you know why the image -- special beast InlineReplacedBox -- forgets(?) about being a child (?) of its containing box? Though the boxes of page n°2 were left alone?

Guess what happens when page 2 is forced to be re-made, too? Right you are: The image is at the place where it belongs.
@liZe

This comment has been minimized.

Copy link
Member

liZe commented Feb 20, 2019

I think that there's a kind of cache for replaced boxes, to avoid having the same image multiple times in memory. But … where is this cache, and is it related to this problem? Good questions.

I think it's the same bug as #387.

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Feb 21, 2019

The image ist stored as a weasyprint.images.RasterImage object in the InlineReplacedBox.replacement, but that's not the problem. It's the InlineReplacedBox.position_y which is reset automagically by re-making the first page.

Contemplating what make_page is expected to do -- collect the blocks that fit on the current page and return the box (resume_at) that must be pushed on the next page...

Question: How can we detect that a box doesnt fit on the current page?
Answer: We try to put it on the current page and check whether it fits.

If I construe the creative code in blocks.py correctly, that's what it actually does.

Conclusion: The miracle is not the incorrect position of the image, but the red-framed div staying at the top of page n°2. And when the red square is replaced by a big black "X", why for heaven's sake, isn't the TextBox affected?

xpaged

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Feb 21, 2019

No mystery at all:

new_box = atomic_box(

All paginated boxes but AtomicInlineLevelBox are copies, created via split_text_box, split_inline_box, or similar.

Ok, there is more within atomic_box(), but to-be-or-not-to-be-a-copy, that's the question.

@mattieusch

This comment has been minimized.

Copy link
Author

mattieusch commented Mar 1, 2019

Would there be a workaround conceivable that can be applied from css or html?
Could it be a solution to always trigger the re-make of the page after the page with the cross-ref to have the image placed correctly?

@liZe liZe closed this in f79110c Mar 1, 2019

@liZe

This comment has been minimized.

Copy link
Member

liZe commented Mar 1, 2019

Ok, there is more within atomic_box(), but to-be-or-not-to-be-a-copy, that's the question.

Let's copy the box. The same problems also happens with replaced blocks, so I've added two tests to check this. If one day we want to avoid the copy, we'll break the tests and try to find another solution then.

@liZe liZe added this to the 46 milestone Mar 1, 2019

@liZe

This comment has been minimized.

Copy link
Member

liZe commented Mar 1, 2019

I think it's the same bug as #387.

For the record: it was.

@Tontyna

This comment has been minimized.

Copy link
Contributor

Tontyna commented Mar 1, 2019

@liZe Good you did it -- me myself had never thought of block_replaced_box_layout

@liZe

This comment has been minimized.

Copy link
Member

liZe commented Mar 1, 2019

@liZe Good you did it -- me myself had never thought of block_replaced_box_layout

I was lucky. As playing with vertical-align was not fun, I've set display: block for the image in the test … and discovered that the bug that was fixed with inline images was still there with block images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.