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

Handle var function in properties with multiple values #1219

Closed
oussama-he opened this issue Sep 28, 2020 · 7 comments · Fixed by #2017
Closed

Handle var function in properties with multiple values #1219

oussama-he opened this issue Sep 28, 2020 · 7 comments · Fixed by #2017
Labels
bug Existing features not working as expected
Milestone

Comments

@oussama-he
Copy link

I'm using weasyprint in one of my projects and I would like to use CSS variables but they don't work correctly.
I have these rules like these in my style.css:

:root {
  --primary-color: #2ea44f;
}

.table thead {
    background-color: var(--primary-color);
    color: #fff;
    text-align: left;
}

.table tbody tr:last-child {
    border-bottom: 2px solid var(--primary-color);
}

The rule .table thead {...} works normally and this rule .table tbody tr:last-child{...} doesn't work and I get a message in the terminal:

WARNING: Ignored `border-bottom: 2px solid var(--primary-color)` at 90:5, invalid value.

Any help, please?
Thank you in advance.

@liZe
Copy link
Member

liZe commented Sep 28, 2020

The problem is that var (and other functions such as attr) don’t work when properties have multiple values.

A workaround for now is to use border-bottom-color: var(--primary-color).

As said in #1165, we have to fully rewrite the way values are calculated, and that’s a lot of work…

@liZe liZe added the bug Existing features not working as expected label Sep 28, 2020
@liZe liZe changed the title Issue with CSS variables Handle functions (var, attr…) in properties with multiple values Sep 28, 2020
@liZe liZe changed the title Handle functions (var, attr…) in properties with multiple values Handle functions (var, attr…) in functions and properties with multiple values Nov 2, 2023
liZe added a commit that referenced this issue Nov 26, 2023
@aleho
Copy link

aleho commented Nov 28, 2023

Just a quick note, this also breaks font-variation-settings: "wght" var(--font-weight-light); where there's not work-around (as far as I know) because the settings have to be written that way.

@liZe
Copy link
Member

liZe commented Nov 28, 2023

Just a quick note, this also breaks font-variation-settings: "wght" var(--font-weight-light); where there's not work-around (as far as I know) because the settings have to be written that way.

Oh, thanks for this quick note. I was happy because we’re close to get var working on shorthand properties, but I "forgot" that some normal properties can also get multiple values.

So, let’s not forget font-variation-settings and its friends (at least font-family, display, some of background-*, border-*-*-radius, border-spacing, bookmark-label, quotes, object-position, marks, hyphenate-limit-chars, transform-origin.)

"Good" news is that attr is not supported by other browsers in properties other than content, so we can safely forget this part for now.

@liZe liZe changed the title Handle functions (var, attr…) in functions and properties with multiple values Handle var function in properties with multiple values Nov 28, 2023
liZe added a commit that referenced this issue Nov 28, 2023
liZe added a commit that referenced this issue Nov 30, 2023
liZe added a commit that referenced this issue Nov 30, 2023
@Dano507
Copy link

Dano507 commented Dec 5, 2023

Hi, I discovered this issue yesterday. From then, I've been looking through the source to find potential fixes.

If I'm understanding correctly, the issue is in the weasyprint/css/validation package, and occurs whenever a css var() function in a property with multiple values is passed to the expander_() call in __init__.py.

expander_ = EXPANDERS.get(name, validate_non_shorthand)
tokens = remove_whitespace(declaration.value)
try:
    # Use list() to consume generators now and catch any error.
    result = list(expander_(base_url, name, tokens))  # ERROR HERE
except InvalidValues as exc:
    validation_error(
        'warning',
        exc.args[0] if exc.args and exc.args[0] else 'invalid value')
    continue

Would evaluating all function tokens (in the tokens array) before the expander_() call be feasible? It seems like a sensible solution, but I don't know how implementing it would work.

@liZe
Copy link
Member

liZe commented Dec 5, 2023

Hi, I discovered this issue yesterday. From then, I've been looking through the source to find potential fixes.

Hi, thanks for the comment!

We’re currently working on this on the functions branch. And you’re right: there were limitations with validators … and a lot of other problems!

The current code (in functions) should work with shorthand properties for "normal" cases. We still have to fix other properties with multiple values (font-variation-settings for example, as explained above) and possibly in other functions. Then we’ll merge and close this issue, hopefully before version 61!

(Now that the code is cleaner, we also secretly hope that it will give us the possibility to handle other functions such as calc() #357, but that’s a secret.)

@Dano507
Copy link

Dano507 commented Dec 5, 2023

The progress looks pretty good so far. I didn't realize that it was this far into being fixed.

we also secretly hope that it will give us the possibility to handle other functions such as calc() #357, but that’s a secret.

(That sounds great. And no worries, I'll keep it a secret)

Thanks for the informative reply, great work so far, and good luck.

liZe added a commit that referenced this issue Dec 8, 2023
liZe added a commit that referenced this issue Dec 8, 2023
liZe added a commit that referenced this issue Dec 10, 2023
liZe added a commit that referenced this issue Dec 27, 2023
liZe added a commit that referenced this issue Dec 27, 2023
liZe added a commit that referenced this issue Dec 27, 2023
@liZe
Copy link
Member

liZe commented Jan 1, 2024

Anyone interested in this feature can test #2017. Feedback is welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Existing features not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants