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.

Sign up for GitHub

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

Jump to bottom

Implement counter-set #1019

Closed
nielsvanvelzen opened this issue Jan 3, 2020 · 7 comments
Closed

Implement counter-set #1019

nielsvanvelzen opened this issue Jan 3, 2020 · 7 comments
Labels
feature New feature that should be supported
Milestone
52

Comments

@nielsvanvelzen
Copy link

nielsvanvelzen commented Jan 3, 2020 •
edited
Loading

I have a big document where the counters are not working as intended. Some counters don't reset when they should.

Unfortunately I was not able to reproduce that exact issue but I was able to get another counting issue that might be caused by the same problem. In this case the counter doesn't increment.

When converting this HTML snippet to PDF:

<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="UTF-8" />
	<title>Counter bug</title>
	<style>
	html {
		counter-reset: h1 h2;
	}

	article h1 {
		counter-increment: h1;
		counter-reset: h2;
	}

	article h2 {
		counter-increment: h2;
		counter-reset: h3;
	}

	article h3 {
		counter-increment: h3;
	}

	article h1::before {
		content: counter(h1) ': ';
	}

	article h2::before {
		content: counter(h1) '.' counter(h2) ': ';
	}

	article h3::before {
		content: counter(h1) '.' counter(h2) '.' counter(h3) ': ';
	}
	</style>
</head>
<body>
	<article>
		<h1>Demo</h1>
	</article>
	<article>
		<h2>Should be 1.1</h2>
	</article>
	<article>
		<h3>Should be 1.1.1</h3>
	</article>
	<article>
		<h3>Should be 1.1.2</h3>
		<h3>Should be 1.1.3</h3>
	</article>
	<article>
		<h3>Should be 1.1.4</h3>
		<h3>Should be 1.1.5</h3>
	</article>
	<article>
		<h2>Should be 1.2</h2>
	</article>
</body>
</html>

The result looks like this:

image

Using version weasyprint 51
@liZe
Copy link
Member

liZe commented Jan 3, 2020

Hello!

It’s not a bug, it’s a feature 😉. You can open this document in other browsers, they will give you the same rendering as WeasyPrint.

You have to reset a counter in a box that contains all the children that use this counter. Otherwise, the counter will only be available for the siblings (and descendants) of your child, but will be automatically reset when quitting your parent.

In your example, article h2 { counter-reset: h3 } creates one counter per article, that’s why it’s reset for each article. You have to define it outside, for example in html. Using html { counter-reset: h1 h2 h3 } fixes your problem.

@liZe liZe closed this as completed Jan 3, 2020
@liZe liZe added the CSS Questions about how to do something with CSS label Jan 3, 2020
@nielsvanvelzen
Copy link
Author

Whoops, my example did indeed not reset it properly. I'll have to find another way to trigger the issue as I do properly reset everything in the document I was talking about.

@liZe
Copy link
Member

liZe commented Jan 3, 2020

Whoops, my example did indeed not reset it properly. I'll have to find another way to trigger the issue as I do properly reset everything in the document I was talking about.

No problem! You can reopen this issue with another example.

@nielsvanvelzen
Copy link
Author

I tried stripping down the document that had the bug to a minimal version and this time I am able to reproduce the bug properly.
This is the HTML:

<!DOCTYPE html>
<html>
<head>
	<title>Counter bug</title>
	<meta charset="UTF-8" />
	<style type="text/css">
	body {
		counter-reset: h2 0 h3 0 h4 0;
	}
	
	body article h2 {
		counter-increment: h2;
		counter-reset: h3 0;
	}

	body article h2::before {
		content: counter(h2) ". ";
	}

	body article h3 {
		counter-increment: h3;
		counter-reset: h4 0;
	}

	body article h3::before {
		content: counter(h2) "." counter(h3) ". ";
	}

	body article h4 {
		counter-increment: h4;
	}

	body article h4::before {
		content: counter(h2) "." counter(h3) "." counter(h4) ". ";
	}
	</style>
</head>
<body>
	<article>
		<h2>1</h2>
	</article>
	<article>
		<h2>2</h2>
	</article>
	<article>
		<h3>2.1</h3>
		<h4>2.1.1</h4>	
		<h4>2.1.2</h4>
		<h4>2.1.3</h4>
	</article>
	<article>
		<h3>2.2</h3>
		<h4>2.2.1</h4>
	</article>
	<article>
		<h2>3</h2>
	</article>
	<article>
		<h3>3.1</h3>
	</article>
	<article>
		<h3>3.2</h3>
	</article>
	<article>
		<h2>4</h2>
		<h3>4.1</h3>
		<h3>4.2</h3>
		<h3>4.3</h3>
	</article>
</body>
</html>

And it looks like this:
image

As you can see, after section 3 we expect 3.1 and 3.2 but instead 3.3 and 3.4 show up.

@liZe
Copy link
Member

liZe commented Jan 6, 2020

As you can see, after section 3 we expect 3.1 and 3.2 but instead 3.3 and 3.4 show up.

Browsers give the same result as WeasyPrint, so I suppose WeasyPrint is right there too 😄.

	body article h2 {
		counter-increment: h2;
		counter-reset: h3 0;
	}

You should use counter-set here, instead of counter-reset. But this property is not widely implemented in browsers, and is not yet in WeasyPrint.

@liZe liZe reopened this Jan 6, 2020
@liZe liZe changed the title Counters not counting properly Implement counter-set Jan 6, 2020
@liZe liZe added feature New feature that should be supported and removed CSS Questions about how to do something with CSS labels Jan 6, 2020
@liZe liZe added this to the 52 milestone Jan 6, 2020
@liZe liZe closed this as completed in cb610e8 Jan 6, 2020
@nielsvanvelzen
Copy link
Author

So if I understand it correctly the place where a reset is done is the "root" in the HTML tree for the counter and the set will update the value in the current tree. Glad it's already fixed. I will try it in the next release.

@liZe
Copy link
Member

liZe commented Jan 7, 2020

So if I understand it correctly the place where a reset is done is the "root" in the HTML tree for the counter and the set will update the value in the current tree.

You understand it correctly. The spec tells that counter-reset instantiates new counters whereas counter-set only manipulates the counter value (and create it only if it doesn’t exist).

Glad it's already fixed. I will try it in the next release.

Have fun!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature New feature that should be supported
Projects
None yet
Milestone
52
Development

No branches or pull requests
2 participants
@liZe @nielsvanvelzen