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

CSS rules not always applied in the same order #69

Closed
h3 opened this issue Nov 2, 2012 · 6 comments
Closed

CSS rules not always applied in the same order #69

h3 opened this issue Nov 2, 2012 · 6 comments

Comments

@h3
Copy link

h3 commented Nov 2, 2012

Usually in CSS if you have the following code:

#content {
    border: 2px solid blue;
}

#content {
    border: 2px solid red;
}

The border of #content should always be red since styles declared last always override previously declared styles.

However this is not the case with xhtml2pdf, with a css declaration like this sometimes the border will be blue, sometimes it will be red.

Since it's an on & off problem, it looks awfully like a threading problem. You can F5 many times and the styles will flips randomly and not necessarily every times.

I know reportlab isn't thread safe so I was pretty sure it was a threading problem, but it doesn't seems so. I tried with the dev server with --nothreading and with apache+wsgi on a single thread and the problem persist in both cases.

I dumped what the CSS parser was seeing and the CSS code was always right, which leaves us only with one possibility. The CSS rules aren't always applied in the same order.

@h3
Copy link
Author

h3 commented Nov 5, 2012

After some digging I can confirm that my suspicions where founded.

Here's self contained way to reproduce the problem:

from xhtml2pdf.w3c.cssParser import CSSParser
from xhtml2pdf.context import pisaCSSBuilder

p = CSSParser()
p.setCSSBuilder(pisaCSSBuilder(mediumSet=["all", "print", "pdf"]))
test = """                                   
#test { border: 1px solid red; }
#test { border: 1px solid blue; }
"""
p.parse(test)

Regardless of the threading mode, some times the result will be this:

({<CSSImmutableSelector 0:1:0:0 *#test >: {'border-bottom-color': 'red',
   'border-bottom-style': 'solid',
   'border-bottom-width': ('1', 'px'),
   'border-left-color': 'red',
   'border-left-style': 'solid',
   'border-left-width': ('1', 'px'),
   'border-right-color': 'red',
   'border-right-style': 'solid',
   'border-right-width': ('1', 'px'),
   'border-top-color': 'red',
   'border-top-style': 'solid',
   'border-top-width': ('1', 'px')},
  <CSSImmutableSelector 0:1:0:0 *#test >: {'border-bottom-color': 'blue',
   'border-bottom-style': 'solid',
   'border-bottom-width': ('1', 'px'),
   'border-left-color': 'blue',
   'border-left-style': 'solid',
   'border-left-width': ('1', 'px'),
   'border-right-color': 'blue',
   'border-right-style': 'solid',
   'border-right-width': ('1', 'px'),
   'border-top-color': 'blue',
   'border-top-style': 'solid',
   'border-top-width': ('1', 'px')}},
 {})

Some other times it will be this:

({<CSSImmutableSelector 0:1:0:0 *#test >: {'border-bottom-color': 'blue',
   'border-bottom-style': 'solid',
   'border-bottom-width': ('1', 'px'),
   'border-left-color': 'blue',
   'border-left-style': 'solid',
   'border-left-width': ('1', 'px'),
   'border-right-color': 'blue',
   'border-right-style': 'solid',
   'border-right-width': ('1', 'px'),
   'border-top-color': 'blue',
   'border-top-style': 'solid',
   'border-top-width': ('1', 'px')},
  <CSSImmutableSelector 0:1:0:0 *#test >: {'border-bottom-color': 'red',
   'border-bottom-style': 'solid',
   'border-bottom-width': ('1', 'px'),
   'border-left-color': 'red',
   'border-left-style': 'solid',
   'border-left-width': ('1', 'px'),
   'border-right-color': 'red',
   'border-right-style': 'solid',
   'border-right-width': ('1', 'px'),
   'border-top-color': 'red',
   'border-top-style': 'solid',
   'border-top-width': ('1', 'px')}},
 {})

Now I suspect that at some point the parser rely on the order of the keys set in a dictionary (which is completely arbitrary) to set the order of the CSS rules.

I'm digging through the source code to find the exact place where the problem occurs, but I'm short on time because I have tight deadlines to respect ..

I'll post updates if I find more.

@h3
Copy link
Author

h3 commented Nov 5, 2012

Not sure if I'm on to something or this is just an oddity, but I found something weird ..

In the example above, p.parse(test) will give the random results as mentioned.

However if you print p.parse(test) instead, you'll get this:

({<CSSImmutableSelector 0:1:0:0 *#test >: {'border-top-style': 'solid', 'border-bottom-style': 'solid', 'border-bottom-width': ('1', 'px'), 'border-right-color': 'red', 'border-bottom-color': 'red', 'border-right-style': 'solid', 'border-right-width': ('1', 'px'), 'border-left-style': 'solid', 'border-top-color': 'red', 'border-top-width': ('1', 'px'), 'border-left-width': ('1', 'px'), 'border-left-color': 'red'}, <CSSImmutableSelector 0:1:0:0 *#test >: {'border-top-style': 'solid', 'border-bottom-style': 'solid', 'border-bottom-width': ('1', 'px'), 'border-right-color': 'blue', 'border-bottom-color': 'blue', 'border-right-style': 'solid', 'border-right-width': ('1', 'px'), 'border-left-style': 'solid', 'border-top-color': 'blue', 'border-top-width': ('1', 'px'), 'border-left-width': ('1', 'px'), 'border-left-color': 'blue'}}, {})

The odd thing is .. that it always print the rules in the right order.

@h3
Copy link
Author

h3 commented Nov 5, 2012

Last update for today ..

Dismiss my last comment, it seems this is due to a quick in the way IPython prints method output.

That said I think I managed to narrow down where the problem is. In w3c/css.py at line 529 there is this class:

class CSSRuleset(dict):
    def findCSSRulesFor(self, element, attrName):
        ruleResults = [ (nodeFilter, declarations) for nodeFilter, declarations in self.iteritems() if (attrName in declarations) and (nodeFilter.matches(element)) ]
        ruleResults.sort()
        return ruleResults

Now if I replace ruleResults.sort() by ruleResults.reverse() the problem is gone for overlapping rules .. but the other (non-overlapping) rules get messed up.

At this point I'm out of idea. I don't have the time to wrap my head around the parser to understand everything that's going on so I might just give up right now and hope my investigation can set somebody on the right track.

@benjaoming
Copy link
Contributor

I would think this is caused by Python 2 ordering by object reference when no other order is specified.

Example:

➜  xhtml2pdf git:(fix-71) ✗ python2
Python 2.7.6 (default, Jun 22 2015, 17:58:13) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> object()
<object object at 0x7f1724e9b080>
>>> object() > object()
False
>>> object() > object()
False
>>> object() > object()
True
>>> 

Creating explicit sorting should fix it.

@benjaoming
Copy link
Contributor

Closing this for now, but it's in favour of the more universal solution proposed in #299.

@fbernhart
Copy link
Contributor

Since Python 3.6 dicts are always insertion-ordered; And as we'll be dropping support for Python < 3.6 very soon, this shouldn't be an issue anymore.

I've tested the given example of @h3 and it's always producing the same, correct output on Python 3.8.5.

So maybe we could also close #299?

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

No branches or pull requests

3 participants