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

Make 'inherit' value work correctly for shorthand properties #1007

Closed
liZe opened this issue Dec 15, 2019 · 6 comments · Fixed by #1524
Closed

Make 'inherit' value work correctly for shorthand properties #1007

liZe opened this issue Dec 15, 2019 · 6 comments · Fixed by #1524
Labels
bug Existing features not working as expected good first issue Issues that can be quite easily solved by Python developers with a good CSS background
Milestone

Comments

@liZe
Copy link
Member

liZe commented Dec 15, 2019

From Cascading and Inheritance:

If a shorthand is specified as one of the CSS-wide keywords, it sets all of its sub-properties to that keyword […].

It should be probably handled by the expander decorator.

@liZe
Copy link
Member Author

liZe commented Dec 15, 2019

For example, text-decoration: inherit is currently rejected. Some other shorthand properties probably set inherit only for one of the expanded properties.

@liZe liZe added conformance bug Existing features not working as expected and removed conformance labels Dec 15, 2019
@Smylers
Copy link
Contributor

Smylers commented Jan 29, 2020

For example, text-decoration: inherit is currently rejected.

This is a regression bug, breaking previously correct behaviour.

We've just tried upgrading WeasyPrint from 44 to 51, and discovered we're now getting a warning that we didn't before:

WARNING: Ignored `text-decoration: inherit` at 136:3, invalid value.

and that some links are now getting underlines that they didn't have before the upgrade. Changing inherit to none restores the previous behaviour of no underlines. Presumably text-decoration-line: inherit would work, too.

This regression is particularly unfortunate for the text-decoration property, which didn't even used to be a shorthand: in CSS2, text-decoration took values directly, and MDN says Edge still doesn't support the individual text-decoration-* properties. So there's likely to be a lot of CSS out there setting text-decoration directly, rather than the individual properties.

@liZe liZe added the good first issue Issues that can be quite easily solved by Python developers with a good CSS background label Jan 29, 2020
@liZe
Copy link
Member Author

liZe commented Jan 29, 2020

This regression is particularly unfortunate for the text-decoration property, which didn't even used to be a shorthand

Yes, that’s exactly why this is a regression: text-decoration used to be a simple property in WeasyPrint, but it’s been transformed into a shortand by 9a73fff.

So there's likely to be a lot of CSS out there setting text-decoration directly, rather than the individual properties.

Yes, it’s infortunate. I’ll try to fix this as soon as I can. If anyone is interested, I can also help someone to fix it, the patch should be pretty small and it’s a good way to learn how WeasyPrint works.

@dannyduncan
Copy link

dannyduncan commented Nov 7, 2020

Hey @liZe I believe I've come across this issue as well. Any updates on a fix? See my error below when using the sample report.css file from the gh-pages branch.

WARNING: Ignored `text-decoration: inherit` at 153:11, invalid value.

Has anyone already addressed? Happy to help if you can provide some guidance.

ckepper added a commit to ckepper/WeasyPrint that referenced this issue Dec 13, 2021
…erties

* skip shorthand expansion for 'inherit' tokens, hereby applying parent styles
@ckepper
Copy link
Contributor

ckepper commented Dec 13, 2021

Hey @liZe, I have just submitted this pull request. Please review

liZe added a commit that referenced this issue Dec 14, 2021
ckepper added a commit to ckepper/WeasyPrint that referenced this issue Dec 14, 2021
ckepper added a commit to ckepper/WeasyPrint that referenced this issue Dec 14, 2021
liZe added a commit that referenced this issue Dec 21, 2021
#1007 Make 'inherit' value work correctly for `word-wrap`
@liZe liZe added this to the 55.0 milestone Dec 21, 2021
liZe pushed a commit that referenced this issue Dec 21, 2021
@liZe
Copy link
Member Author

liZe commented Dec 21, 2021

#1524 is the draft pull request to solve this issue.

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 good first issue Issues that can be quite easily solved by Python developers with a good CSS background
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants