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

string() doesn't work in pseudo-elements #723

Open
Smylers opened this Issue Oct 31, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@Smylers
Contributor

Smylers commented Oct 31, 2018

The string(foo) function returns the contents of the specified named string in page margin boxes, but just returns the empty string in the main page contents — specifically in :before and :after pseudo-elements, which are the other place where the content: property can be set to arbitrary text.

Demo:

<!DOCTYPE html>
<title>content: string Check</title>
<style>
body { string-set: title 'flrbbbb' }
@page { @top-left  { content: '[' string(title) ']' } }
p:after            { content: '{' string(title) '}' }
</style>

<p>After: 

The page header shows the expected “[flrbbbb]”, but the paragraph just says “After: {}”, with nothing in the curly brackets.

The draft spec says “Named strings are a convenient way to pull metadata out of the document for insertion into headers and footers” — but I can't find anything that says they should only work there, rather than everywhere that content: can be used.

(However, I haven't yet found a reason for doing this in a real document; I encountered the limitation simply while trying to construct a minimal demo for Issue #722.)

@liZe liZe added the bug label Oct 31, 2018

@liZe liZe added this to the 43 milestone Oct 31, 2018

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Oct 31, 2018

@SimonSapin's statement on GCPM and string() still holds.

In the spec the function is only mentioned when they talk about content in the page (margins), so my understanding of the spec is that string() is only supported in @page context. That's why you can blame me for this comment line and excluding condition in build.py.

But of course, @Smylers is right, too: Nowhere in the spec the use of string() outside of @page is forbidden explicitly -- though I have no idea how to implement the (page-related!) specifiers first | start | last | first-except in the document correctly.

@liZe you're sure that this is a bug?

@Smylers

This comment has been minimized.

Contributor

Smylers commented Nov 1, 2018

@SimonSapin's statement on GCPM and string() still holds.

Ah, I hadn't seen that. Thanks.

In the spec the function is only mentioned when they talk about content in the page (margins), so my understanding of the spec is that string() is only supported in @page context.

OK. In that case, please could it actually be invalid, like any other unknown function is?

Changing string() to, say, stringX() in the above demo causes the pseudo-element's content to be completely empty (rather than ‘{}’ from the constants either side of the ignored function), and the emission of this warning:

WARNING: Ignored `content: "{" stringX(title) "}" ` at 4:22, invalid value.

If string() in pseudo-elements also did that, it'd be clearer that it isn't intended to work.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Nov 1, 2018

Emitting a WARNING is no problem, only the at col:line part telling where to look in the css file is -- @liZe correct me if I'm wrong -- impossible.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Nov 1, 2018

Ah, and I remember there was a discussion whether to throw away the whole content or only the invalid string() part

@liZe liZe added feature and removed bug labels Nov 2, 2018

@liZe

This comment has been minimized.

Member

liZe commented Nov 2, 2018

@SimonSapin's statement on GCPM and string() still holds.

True, I didn't remember that. But the feature may be useful, and nothing seems to prevent this in the spec.

For example, it’s not clear to me whether the string() function of the content property is valid in contexts other than on page-margin boxes (e.g. ::before or ::after pseudo-elements) and what the second argument means in this case.

There's no logical problem now with the second argument, it should behave just as it does when string is used in page margins.

OK. In that case, please could it actually be invalid, like any other unknown function is?

Yes, we should.

only the at col:line part telling where to look in the css file is -- @liZe correct me if I'm wrong -- impossible.

It's actually possible, it's even already done for most of CSS errors 😄. What's impossible (without lxml) is to get the position in HTML files.

Ah, and I remember there was a discussion whether to throw away the whole content or only the invalid string() part

We should invalidate the whole rule if we decide that string is not allowed (I hope that it's what I said in the previous discussion).

I'll invalidate the rule and keep this issue open to add the feature.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Nov 2, 2018

It's actually possible,
I'll invalidate the rule

@liZe Looking forward how you'll implement that 😏

Reason-for-smirk:
content() in css.validation.properies resp. get_content_list() in css.utils has no idea to what kind of css rule (page, pseudo element, real element) the content property belongs.

We know the element (but not the col:line where its css is defined) in computed_values.py -- I recall a version of computed_values.py where I discarded parts of the content_list depending on the element , but this "validation" was removed by ... authority ... and moved to compute_content_list() in build.py

@liZe

This comment has been minimized.

Member

liZe commented Nov 2, 2018

@liZe Looking forward how you'll implement that

😆

liZe added a commit that referenced this issue Nov 2, 2018

@liZe

This comment has been minimized.

Member

liZe commented Nov 2, 2018

We should invalidate the whole rule if we decide that string is not allowed (I hope that it's what I said in the previous discussion).

Of course, that's what we should do if we had the possibility to know if the function is called in the page margin box or elsewhere.

content() in css.validation.properies resp. get_content_list() in css.utils has no idea to what kind of css rule (page, pseudo element, real element) the content property belongs.

Well… You're right.

A solution would be to have a list of validation functions for each context (page, page margin, page content). It also would be useful to reject properties that are not allowed in a particular context (here are the properties allowed for page and for page margin).

but this "validation" was removed by ... authority ... and moved to compute_content_list() in build.py

If we decide that string() is not allowed in the rule, then we have to refuse the whole rule during validation, not during value computation, to allow the use of a fallback rule defined earlier.

For example:

p::before {
  content: 'lol';
  content: '[' string(title) ']';
}

should keep 'lol' if we decide to refuse string(), and that's what's done during validation. After validation it's too late to discard the whole second rule.

The current implementation (and 36ef44c keeps it with just a warning message) allows string() but makes it behave like an empty string.

@liZe liZe removed this from the 43 milestone Nov 2, 2018

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Nov 2, 2018

Not to mention the question how to tell invalid from not implemented.

Not to mention the problem that while parsing the css one cannot be sure to which kind of element the rule will be applied -- most obvious cases: id and style attributes happen in the html.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Nov 2, 2018

Ah, stupid me: As @liZe pointed out the rule's context -- page, page margin, page content -- should be sufficient for enhanced css validation and this context is well-known in the rule.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Nov 14, 2018

py-weasyprint: Update to 43.
Version 43
----------

Released on 2018-11-09.

Bug fixes:

* `#726 <https://github.com/Kozea/WeasyPrint/issues/726>`_:
  Make empty strings clear previous values of named strings
* `#729 <https://github.com/Kozea/WeasyPrint/issues/729>`_:
  Include tools in packaging

This version also includes the changes from unstable rc1 and rc2 versions
listed below.

Version 43rc2
-------------

Released on 2018-11-02.

**This version is experimental, don't use it in production. If you find bugs,
please report them!**

Bug fixes:

* `#706 <https://github.com/Kozea/WeasyPrint/issues/706>`_:
  Fix text-indent at the beginning of a page
* `#687 <https://github.com/Kozea/WeasyPrint/issues/687>`_:
  Allow query strings in file:// URIs
* `#720 <https://github.com/Kozea/WeasyPrint/issues/720>`_:
  Optimize minimum size calculation of long inline elements
* `#717 <https://github.com/Kozea/WeasyPrint/issues/717>`_:
  Display <details> tags as blocks
* `#691 <https://github.com/Kozea/WeasyPrint/issues/691>`_:
  Don't recalculate max content widths when distributing extra space for tables
* `#722 <https://github.com/Kozea/WeasyPrint/issues/722>`_:
  Fix bookmarks and strings set on images
* `#723 <https://github.com/Kozea/WeasyPrint/issues/723>`_:
  Warn users when string() is not used in page margin


Version 43rc1
-------------

Released on 2018-10-15.

**This version is experimental, don't use it in production. If you find bugs,
please report them!**

Dependencies:

* Python 3.4+ is now needed, Python 2.x is not supported anymore
* Cairo 1.15.4+ is now needed, but 1.10+ should work with missing features
  (such as links, outlines and metadata)
* Pdfrw is not needed anymore

New features:

* `Beautiful website <https://weasyprint.org>`_
* `#579 <https://github.com/Kozea/WeasyPrint/issues/579>`_:
  Initial support of flexbox
* `#592 <https://github.com/Kozea/WeasyPrint/pull/592>`_:
  Support @font-face on Windows
* `#306 <https://github.com/Kozea/WeasyPrint/issues/306>`_:
  Add a timeout parameter to the URL fetcher functions
* `#594 <https://github.com/Kozea/WeasyPrint/pull/594>`_:
  Split tests using modern pytest features
* `#599 <https://github.com/Kozea/WeasyPrint/pull/599>`_:
  Make tests pass on Windows
* `#604 <https://github.com/Kozea/WeasyPrint/pull/604>`_:
  Handle target counters and target texts
* `#631 <https://github.com/Kozea/WeasyPrint/pull/631>`_:
  Enable counter-increment and counter-reset in page context
* `#622 <https://github.com/Kozea/WeasyPrint/issues/622>`_:
  Allow pathlib.Path objects for HTML, CSS and Attachment classes
* `#674 <https://github.com/Kozea/WeasyPrint/issues/674>`_:
  Add extensive installation instructions for Windows

Bug fixes:

* `#558 <https://github.com/Kozea/WeasyPrint/issues/558>`_:
  Fix attachments
* `#565 <https://github.com/Kozea/WeasyPrint/issues/565>`_,
  `#596 <https://github.com/Kozea/WeasyPrint/issues/596>`_,
  `#539 <https://github.com/Kozea/WeasyPrint/issues/539>`_:
  Fix many PDF rendering, printing and compatibility problems
* `#614 <https://github.com/Kozea/WeasyPrint/issues/614>`_:
  Avoid crashes and endless loops caused by a Pango bug
* `#662 <https://github.com/Kozea/WeasyPrint/pull/662>`_:
  Fix warnings and errors when generating documentation
* `#666 <https://github.com/Kozea/WeasyPrint/issues/666>`_,
  `#685 <https://github.com/Kozea/WeasyPrint/issues/685>`_:
  Fix many table layout rendering problems
* `#680 <https://github.com/Kozea/WeasyPrint/pull/680>`_:
  Don't crash when there's no font available
* `#662 <https://github.com/Kozea/WeasyPrint/pull/662>`_:
  Fix support of some align values in tables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment