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

Dont crash when element/target-collector is None #621

Merged
merged 1 commit into from
Apr 30, 2018

Conversation

Tontyna
Copy link
Contributor

@Tontyna Tontyna commented Apr 23, 2018

Concerning compute_attr_function(): Why not return the fallback?

Concerning _content_list(): target_collector is None when called in @page context. Not shure whether the spec forbids target-* in that context. In any case: Since all the anchors are already collected when we hit compute() within the page margins, no harm is done by not calling target_collector.collect_computed_target(). A css like that works as expected:

@page {
  @top-center {
    content: "countervalue at #id: " target-counter("#id", countername);
  }

That is: Page margins gain access to targets in the document without a detour via string-set/string(). OMG, I fear thats not allowed by the spec...

BTW: With your cleanups it's almost a pleasure to read the code 😃

@liZe
Copy link
Member

liZe commented Apr 30, 2018

Concerning compute_attr_function(): Why not return the fallback?

Returning some type when the caller is waiting for another type may break the caller. Oh, and I'm pretty sure nobody actually cares!

Page margins gain access to targets in the document without a detour via string-set/string(). OMG, I fear thats not allowed by the spec...

😝

Looks like it's not forbidden.

BTW: With your cleanups it's almost a pleasure to read the code

It's not perfect but it's much better like this 😉.

One last question: when is computer.element None?

@liZe liZe merged commit b7c5b1c into Kozea:target-collector Apr 30, 2018
@Tontyna
Copy link
Contributor Author

Tontyna commented May 1, 2018

Oh, and I'm pretty sure nobody actually cares!

Right you are.

computer.element is None when called (via computed_from_cascaded) by
layout.inlies.first_letter_to_box(), layout.pages.make_margin_boxes.make_box() and formatting_structures.boxes.Box.anonymous_from()

@Tontyna Tontyna deleted the contrib2target branch May 1, 2018 20:44
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