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

Major changes for WeasyPrint #5

Merged
merged 9 commits into from
Jul 7, 2017
Merged

Major changes for WeasyPrint #5

merged 9 commits into from
Jul 7, 2017

Conversation

liZe
Copy link
Member

@liZe liZe commented Jul 3, 2017

No description provided.

@liZe
Copy link
Member Author

liZe commented Jul 6, 2017

Main changes:

  • ElementWrapper doesn't try to mimic ElementTree's Element, Element's attributes and methods (get) are not available from the wrapper,
  • a base_url attribute has been added to the wrapper and is automatically taken from the parent if needed,
  • match returns (specificity, order, pseudo, payload) instead of just the payload.

Fixes (both issues found by Acid2):

  • selectors with pseudo-classes missing the universal selectors (like html :hover) are now parsed correctly,
  • the declaration order is explicitely kept to fix cases where two selectors have the same global specificity but their last simple selectors don't have the same specificity:
<style>
  p .class { color: red }
  .class p { color: blue }
</style>
<p class="class">
  <p class="class">
    This text was red and is now blue.
  </p>
</p>

I'm not really happy with this API, especially with what's returned by match. The goal of this PR is to have a 0.1 version released quickly, use it in WeasyPrint and have a discussion with other users interested in cssselect2 (about the API, #1 and everything else) with a real-life working example.

Selectors level 4 (in cssselect2) and Named pages (in WeasyPrint) are some of the next challenges 😉.

@SimonSapin is that OK for you?

@SimonSapin
Copy link
Member

Feel free to do whatever’s good for WeasyPrint with this repo :) Maybe even move into the WeasyPrint repository? You could still publish packages to PyPI separately if desired. (BTW I just gave you PyPI access.)

I haven’t looked at the diffs, but based on your "main changes" description:

  • I think ElementWrapper should go further and not be a wrapper at all. Maybe eliminate the children list and instead have previous_siblings, next_sibling, first_child, and last_child links to other elements directly, if that makes matching easier.
  • Unless you want to support xml:base (which is fairly obscure), the base URL is the same for an entire documents. Maybe it doesn’t need to be stored on each element?
  • We’ve discussed the match return type on IRC, I think this change should not be needed if you have multiple Matchers. But whatever you find convenient.
  • I think declaration order was preserved by using a stable sort. Was that buggy?

@liZe
Copy link
Member Author

liZe commented Jul 6, 2017

Maybe even move into the WeasyPrint repository? You could still publish packages to PyPI separately if desired. (BTW I just gave you PyPI access.)

I'd like to see if anyone else is interested in cssselect2 before merging it into WeasyPrint.

I think ElementWrapper should go further and not be a wrapper at all. Maybe eliminate the children list and instead have previous_siblings, next_sibling, first_child, and last_child links to other elements directly, if that makes matching easier.

I agree, I'll do that later.

Unless you want to support xml:base (which is fairly obscure), the base URL is the same for an entire documents. Maybe it doesn’t need to be stored on each element?

Yes, I've added it because it was easier, I should remove it before the release (I think that the hard part is already done in WeasyPrint).

We’ve discussed the match return type on IRC, I think this change should not be needed if you have multiple Matchers. But whatever you find convenient.

I think that having one matcher helps to keep the code simple in WeasyPrint for now.

I think declaration order was preserved by using a stable sort. Was that buggy?

Yes, because in match the relevant_selectors are added in an order related to the specificity of their last simple selector.

@liZe liZe merged commit 7465561 into master Jul 7, 2017
@liZe liZe deleted the weasyprint branch July 7, 2017 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants