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

Table auto-layout differences compared to browsers #906

Closed
rtpg opened this issue Jul 22, 2019 · 5 comments

Comments

@rtpg
Copy link
Contributor

commented Jul 22, 2019

So I have this HTML

<style>
td, div {
  border: 1px solid black;
}
</style>
<div style="width: 1cm;">
  Hi this is 1cm long
</div>

<div style="width: 1cm; white-space: nowrap">
  Hi this is also 1cm long
</div>

<table style="width: 10cm">
  <tr>
    <td style="width: 1cm;">
      This is 1cm
    </td>
    <td style="width: 1cm; white-space: nowrap">
      This is not 1cm
    </td>
    <td>
      This will take the rest
    </td>
  </tr>
</table>

(I have a nice fiddle for this https://jsfiddle.net/4wr36ouh/)

Here I have some 1cm divs, along with some tables that have 1cm columns (or rather, they should).

the white-space: nowrap property on the second table cell means that browsers end up expanding this column/cell to be more than 1cm long.

However, in weasyprint, this property causes the content to escape the cell, with the cell staying 1cm. See this PDF test_pdf.pdf

Now, the auto-layout spec is User Agent-specific.... I don't know if this is just an incidental issue in the weasyprint algorithm leading to the difference, or if it's a fundamental difference in the interpretation of the width property on table cells, but the browser results tend to look better and feel like a nice target to emulate.

I'm going to try looking into this a bit more later, I believe there could be a very small patch that unifies behavior here. But I would like to know if there's any intentionality in this behavior that I should be aware about?

@liZe liZe added the bug label Jul 22, 2019

@liZe

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

But I would like to know if there's any intentionality in this behavior that I should be aware about?

No, there's not. The problem is real and probably caused by a wrong value for the min width of table cells.

@rtpg

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

Yeah looking into this a bit more I found that content_min_width for the table cell was returning just the width, instead of a content-aware result that took into account that would adapt to the non-wrapping.

@liZe

This comment has been minimized.

Copy link
Member

commented Jul 23, 2019

Yeah looking into this a bit more I found that content_min_width for the table cell was returning just the width, instead of a content-aware result that took into account that would adapt to the non-wrapping.

That's right, we treat cells as we treat other containing blocks when calculating minimum content width. We should check that content doesn't escape from the block for cells instead.

There's an extra if condition to add at the beginning of min_content_width. @rtpg Are you interested in adding this code? I can help if you want, or I can add it myself if you're not interested, no problem!

@rtpg

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

@liZe I was messing around on this problem all day, trying various things to get the result I needed here (I'm on JST), but I'm no longer confident in the kind of patch I'm creating to solve this problem. If you could take a whack at this I'd greatly appreciate it.

I opened up a PR with my work (I think the test itself is pretty important, not at all sure if I'm tackling the issue at the right spot though)

(As an aside, the code base is really interesting to walk through. This is a cool project!)

@liZe liZe closed this in 8d10c01 Jul 24, 2019

@liZe liZe added this to the 49 milestone Jul 24, 2019

@liZe

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Thanks a lot for your PR, I've simplified the test and included it in the fix.

As you can see in 8d10c01, the fix was to add specific min_content_width and max_content_width checks for table cells, because unlike other blocks they have to respect the minimum content size when it's greater than the specified width.

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