Allow overriding font-based normal line-height

[moben]

For koreader/koreader#12483, see description there

---

This change is 

[poire-z]

May want to rebase just to be clean and sure.

[moben]

Moved to -cr-only-if: line-height-normal, as suggested in koreader/koreader#12483.

It's a bit annoying that the parent style needs to be passed through everywhere, but it's all coming from lvrend's setNodeStyle, which does know the parent style. But if I understand things correctly that's not really avoidable unless most of what setNodeStyle does (i.e. computing/inheriting the actual values) is moved to LVCssDeclaration::parse. Otherwise -cr-only-if can always have trouble matching if the actual value is only computed later.

[poire-z]

I'll read your posts and review better tomorrow.

unless most of what setNodeStyle does (i.e. computing/inheriting the actual values) is moved to LVCssDeclaration::parse.

These are really 2 different things: LVCssDeclaration::parse parses a stylesheet way earlier before we get to act on nodes. Which is what setNodeStyle does: applying.

Otherwise -cr-only-if can always have trouble matching if the actual value is only computed later.

I suggested using -cr-hint: late;, which you did use. It's supposed to get that declaration applied (by setNodeStyle) after all others, so normally after all those that set line-height:.
But I'm not sure (haven't looked) if it applies after line-height:inherit in case the node inherits normal.

[moben]

These are really 2 different things: LVCssDeclaration::parse parses a stylesheet way earlier before we get to act on nodes. Which is what setNodeStyle does: applying.

Sorry, I got that wrong when writing the comment, I meant LVCssDeclaration::apply of course, which is where the condition for -cr-only-if is checked. There you still have line-height: inherit when that is checked because setNodeStyle calls apply and then does all the inheritance.

I'm open to suggestions for a better way to do this, but I don't see how. -cr-hint: late declarations could set something to inherit again, so you need to run inheritance after them. I guess the whole inheritance / computed value part of setNodeStyle could be it's own function, so it can be re-used when checking -cr-only-if instead of open coding it, which would be a slight improvement?

[poire-z]

I'm open to suggestions for a better way to do this, but I don't see how. -cr-hint: late declarations could set something to inherit again, so you need to run inheritance after them.

-cr-hint: late is our custom stuff, we don't need to allow line-height: inherit to work, we can limit its feature anyway we want, to keep the code simpler.

I guess the whole inheritance / computed value part of setNodeStyle could be it's own function, so it can be re-used when checking -cr-only-if instead of open coding it, which would be a slight improvement?

All the style stuff/setNodeStyle and the rendering methods stuff is quite fragile, there are 2 different code paths using it (initial loading with style computer as we build the dom, vs later rerendering where the dom is already present) and there had been many issues keeping that consistent and the various hashs that detects that identical. So, the less stuff we change the better.

If there is any change we get a parent node with computed style line-height: normal, and its child node gets a computed style of line-height: inherit at the time your -cr-hint:late are checked, I would just add a check in the function to get the parent style and see if it is "normal".
Oh, just looking at the code, that's what you already do :)

But you don't need to pass parent_style alll along, as you're nearly the only user of that. Please limit the changes to the minimum.
You can get to it from the node itself, with getParentNode()->getStyle().
cssd_cr_apply_func (just below what you added) / apply_cr_apply_func / cr_apply_func_css_text_align_center_if_parent_text_align_initial already do that.

[poire-z]

We'll wait for this to reach koreader frontend to merge koreader/koreader#12483.