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

Some CSS attribute selectors and pseudoclasses fixes #303

Merged
merged 3 commits into from
Aug 3, 2019

Conversation

poire-z
Copy link
Contributor

@poire-z poire-z commented Aug 3, 2019

See individual commit messages for details.
First commit fixes the another strange thing noticed in #276 (comment):

There's another strange thing with this book: when you change some rendering parameters (font size, margins), it triggers a Styles have changed in such a way that fully reloading the document may be needed for a correct rendering. popup - which I've never seen happening before when ... styles have no reason to have changed :|

Second commit fixes attribute selectors that should not match when there is no such attribute. It should avoid some uneeded expensive work, as noticed in #276 (comment).
Third commit makes the parsing of attribute selectors more correct that what was done in 3430e51.

Before 2nd and 3rd commits, this HTML:

<html>
<head>
<style>
div[tutu= i], blockquote { font-weight: bold; }
div[toto=], blockquote { font-style: italic; } /* should be fully skipped as invalid */
div[tutu="" i] { color: red; }
div[toto=""] { background-color: yellow; }
div[tutu="def i"] { color: orange; }
div[tutu="def" i] { text-decoration: underline; }
div[toto=abc i] { color: yellow; }
</style>
</head>
<body>
 <div>Some text no toto no tutu</div>
 <div toto="">Some text with empty toto</div>
 <div tutu="">Some text with empty tutu</div>
 <div toto="abc">Some text with non empty toto</div>
 <div tutu="def">Some text with non empty tutu</div>
 <blockquote>Some text in blockquote</blockquote>
</body>
</html>

would render as that (Firefox on the left, KOReader on the right):
before

With 2nd and 3rd commits, we now render as Firefox:
after

In the initial loading phase, when the HTML is parsed and the DOM
is being built, and styles computed (cause we know enough: the node
class, all the parents), when meeting the nodes as they are parsed,
the current node is... always the last child (which might be the only
thing we don't know yet...). So, some CSS pseudoclasses like:
    :last-child :last-of-type
    :nth-last-child() :nth-last-of-type()
    :only-child :only-of-type
might be matched on more nodes that they should.
So, when meeting such a pseudoclass, we flag that a re-render
is needed, which will be done on the fully made DOM, so they
are matched correctly.
An empty string attribute selector (div[attr=""]) should not
match a node when that attribute is absent.
(getAttributeValue() returns an empty string when the attribute
is absent, that would make us do some work with it.)
This should also avoid the expensive lowercase() needed by our
epub.css attribute selectors (eg. br[clear=left i]) when there
is no clear= attribute on the <br> we meet.
The ' i' flag should be outside the parenthesis with the attribute
value:
div[foo="bar i"] should not match foo="bar" nor foo="BAR", but
only foo="bar i".

Also, "div[foo=], blockquote {...}" is invalid and should make
the whole declaration discarded (including the blockquote).
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

👍

@poire-z
Copy link
Contributor Author

poire-z commented Aug 3, 2019

Strangely, the commits didn't reach github in the same order they are on my side :/:

$ git log --oneline
9a1bbd01 CSS: attribute selectors: fix parsing of case insensitive flag
c187c0f3 CSS: attribute selectors: don't check when there is no such attribute
709ae63b Force a re-render when :last-child & other pseudoclasses are met
3433c077 Minor zero-initalization cleanups (#301)
[...]

So, switch 2nd and 3rd when reading first post. They got re-ordered correctly by rebase&merge when merged into master...

@poire-z poire-z merged commit 5e940f1 into koreader:master Aug 3, 2019
@poire-z poire-z deleted the css_attr_last_child_fixes branch August 3, 2019 17:55
Frenzie pushed a commit to koreader/koreader that referenced this pull request Aug 3, 2019
Includes koreader/crengine#303:
- Force a re-render when :last-child & other pseudoclasses are met
- CSS: attribute selectors: don't check when there is no such attribute
- CSS: attribute selectors: fix parsing of case insensitive flag
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
…der#5178)

Includes koreader/crengine#303:
- Force a re-render when :last-child & other pseudoclasses are met
- CSS: attribute selectors: don't check when there is no such attribute
- CSS: attribute selectors: fix parsing of case insensitive flag
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.

None yet

2 participants