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

Add support for concatenating var() functions in 'content' declarations #1165

Merged
merged 6 commits into from
Sep 4, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 53 additions & 32 deletions weasyprint/css/computed_values.py
Original file line number Diff line number Diff line change
@@ -176,6 +176,46 @@ def decorator(function):
return decorator


def compute_variable(value, name, computed, base_url, parent_style):
from .validation.properties import PROPERTIES
already_computed_value = False

if value and isinstance(value, tuple) and value[0] == 'var()':
variable_name, default = value[1]
computed_value = _resolve_var(computed, variable_name, default)
if computed_value is None:
new_value = None
else:
prop = PROPERTIES[name.replace('_', '-')]
if prop.wants_base_url:
new_value = prop(computed_value, base_url)
else:
new_value = prop(computed_value)

# See https://drafts.csswg.org/css-variables/#invalid-variables
if new_value is None:
try:
computed_value = ''.join(
token.serialize() for token in computed_value)
except BaseException:
pass
LOGGER.warning(
'Unsupported computed value `%s` set in variable `%s` '
'for property `%s`.', computed_value,
variable_name.replace('_', '-'), name.replace('_', '-'))
if name in INHERITED and parent_style:
already_computed_value = True
value = parent_style[name]
else:
already_computed_value = name not in INITIAL_NOT_COMPUTED
value = INITIAL_VALUES[name]
elif isinstance(new_value, list):
value, = new_value
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content property handler always returns a list regardless of input. In this case, the handler will only ever get a computed variable declaration, and thus I unpack the computed value again after getting it back from the handler.

else:
value = new_value
return value, already_computed_value


def compute(element, pseudo_type, specified, computed, parent_style,
root_style, base_url, target_collector):
"""Create a dict of computed values.
@@ -193,7 +233,6 @@ def compute(element, pseudo_type, specified, computed, parent_style,
:param target_collector: A target collector used to get computed targets.

"""
from .validation.properties import PROPERTIES

computer = {
'is_root_element': parent_style is None,
@@ -218,37 +257,19 @@ def compute(element, pseudo_type, specified, computed, parent_style,
function = getter(name)
already_computed_value = False

if value and isinstance(value, tuple) and value[0] == 'var()':
variable_name, default = value[1]
computed_value = _resolve_var(computed, variable_name, default)
if computed_value is None:
new_value = None
else:
prop = PROPERTIES[name.replace('_', '-')]
if prop.wants_base_url:
new_value = prop(computed_value, base_url)
else:
new_value = prop(computed_value)

# See https://drafts.csswg.org/css-variables/#invalid-variables
if new_value is None:
try:
computed_value = ''.join(
token.serialize() for token in computed_value)
except BaseException:
pass
LOGGER.warning(
'Unsupported computed value `%s` set in variable `%s` '
'for property `%s`.', computed_value,
variable_name.replace('_', '-'), name.replace('_', '-'))
if name in INHERITED and parent_style:
already_computed_value = True
value = parent_style[name]
else:
already_computed_value = name not in INITIAL_NOT_COMPUTED
value = INITIAL_VALUES[name]
else:
value = new_value
if value:
converted_to_list = False

if not isinstance(value, list):
converted_to_list = True
value = [value]
Copy link
Author

@mikevoets mikevoets Jul 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multi values properties (for now only content property classifies as such) return a list that possibly includes variable functions among their declaration values. Therefore, isinstance(value, tuple) and value[0] == 'var()' would never be true for that situation.

I fixed this by looping over all the values and compute the variables when there are variable functions. I temporarily convert non-list declaration values to a list to do this elegantly.


for i, v in enumerate(value):
value[i], already_computed_value = \
compute_variable(v, name, computed, base_url, parent_style)

if converted_to_list:
value, = value

if function is not None and not already_computed_value:
value = function(computer, name, value)
5 changes: 5 additions & 0 deletions weasyprint/css/utils.py
Original file line number Diff line number Diff line change
@@ -699,6 +699,11 @@ def get_content_list_token(token, base_url):
if string is not None:
return string

# <var>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here a variable function in a content declaration is acknowledged and returned accordingly.

var = check_var_function(token)
if var is not None:
return var

# contents
if get_keyword(token) == 'contents':
return ('content', 'text')
16 changes: 10 additions & 6 deletions weasyprint/css/validation/properties.py
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
PREFIX = '-weasy-'
PROPRIETARY = set()
UNSTABLE = set()
MULTIVAL_PROPERTIES = set()

# Yes/no validators for non-shorthand properties
# Maps property names to functions taking a property name and a value list,
@@ -33,7 +34,7 @@
# Validators

def property(property_name=None, proprietary=False, unstable=False,
wants_base_url=False):
multiple_values=False, wants_base_url=False):
"""Decorator adding a function to the ``PROPERTIES``.

The name of the property covered by the decorated function is set to
@@ -69,6 +70,8 @@ def decorator(function):
PROPRIETARY.add(name)
if unstable:
UNSTABLE.add(name)
if multiple_values:
MULTIVAL_PROPERTIES.add(name)
return function
return decorator

@@ -89,10 +92,11 @@ def validate_non_shorthand(base_url, name, tokens, required=False):
if not required and name not in PROPERTIES:
raise InvalidValues('property not supported yet')

for token in tokens:
var_function = check_var_function(token)
if var_function:
return ((name, var_function),)
if name not in MULTIVAL_PROPERTIES:
Copy link
Author

@mikevoets mikevoets Jul 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary because a content declaration is a special case since its property handler returns a list, as opposed to every other property. Currently if one of the rules in content is a variable function, it would only parse that variable function and return it from the function here, without processing the other rules. This is why I have added a special case condition here.

I cleaned it up by declaring the content property as a multi values property.

for token in tokens:
var_function = check_var_function(token)
if var_function:
return ((name, var_function),)

keyword = get_single_keyword(tokens)
if keyword in ('initial', 'inherit'):
@@ -454,7 +458,7 @@ def clip(token):
return ()


@property(wants_base_url=True)
@property(multiple_values=True, wants_base_url=True)
def content(tokens, base_url):
"""``content`` property validation."""
# See https://www.w3.org/TR/css-content-3/#content-property
1 change: 1 addition & 0 deletions weasyprint/document.py
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@
import warnings

import cairocffi as cairo

from weasyprint.layout import LayoutContext

from . import CSS
2 changes: 1 addition & 1 deletion weasyprint/layout/preferred.py
Original file line number Diff line number Diff line change
@@ -718,7 +718,7 @@ def flex_max_content_width(context, box, outer=True):

def trailing_whitespace_size(context, box):
"""Return the size of the trailing whitespace of ``box``."""
from .inlines import split_text_box, split_first_line
from .inlines import split_first_line, split_text_box

while isinstance(box, (boxes.InlineBox, boxes.LineBox)):
if not box.children:
8 changes: 6 additions & 2 deletions weasyprint/tests/test_counters.py
Original file line number Diff line number Diff line change
@@ -245,25 +245,29 @@ def test_counters_8():
def test_counter_styles_1():
assert_tree(parse_all('''
<style>
body { counter-reset: p -12 }
body { --var: 'Counter'; counter-reset: p -12 }
p { counter-increment: p }
p:nth-child(1):before { content: '-' counter(p, none) '-'; }
p:nth-child(2):before { content: counter(p, disc); }
p:nth-child(3):before { content: counter(p, circle); }
p:nth-child(4):before { content: counter(p, square); }
p:nth-child(5):before { content: counter(p); }
p:nth-child(6):before { content: var(--var) ':' counter(p); }
p:nth-child(7):before { content: counter(p) ':' var(--var); }
</style>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
<p></p>
'''), [
('p', 'Block', [
('p', 'Line', [
('p::before', 'Inline', [
('p::before', 'Text', counter)])])])
for counter in '-- • ◦ ◾ -7'.split()])
for counter in '-- • ◦ ◾ -7 Counter:-6 -5:Counter'.split()])


@assert_no_logs
19 changes: 19 additions & 0 deletions weasyprint/tests/test_variables.py
Original file line number Diff line number Diff line change
@@ -11,6 +11,8 @@
from ..css.properties import KNOWN_PROPERTIES
from .test_boxes import render_pages as parse

SIDES = ('top', 'right', 'bottom', 'left')


def test_variable_simple():
page, = parse('''
@@ -83,6 +85,23 @@ def test_variable_chain():
assert paragraph.width == 10


def test_variable_partial_1():
page, = parse('''
<style>
html { --var: 10px }
div { margin: 0 0 0 var(--var) }
</style>
<div></div>
''')
html, = page.children
body, = html.children
div, = body.children
assert div.margin_top == 0
assert div.margin_right == 0
assert div.margin_bottom == 0
assert div.margin_left == 10


def test_variable_initial():
page, = parse('''
<style>