Merge lp:~osomon/elisa/css_property_no_unit into lp:elisa

Proposed by Olivier Tilloy
Status: Merged
Merged at revision: 1618
Proposed branch: lp:~osomon/elisa/css_property_no_unit
Merge into: lp:elisa
Diff against target: 26 lines (+14/-2)
1 file modified
elisa-plugins/elisa/plugins/pigment/widgets/ (+14/-2)
To merge this branch: bzr merge lp:~osomon/elisa/css_property_no_unit
Reviewer Review Type Date Requested Status
Florian Boucault Approve
Jonathan Rauprich Approve
Review via email:

Description of the change

This branch addresses bug #379409.
Recent versions of the parser in CSSUtils (≥ 0.9.6, e.g. as found in Ubuntu 10.04) will trim the unit for zero values: "top_height: 0px;" will become "top_height: 0;".
The issue is with the PiecePanel widget that always expects ValueWithUnits for its attributes.
The proposed solution restores the unit for values that don't have it in update_style_properties().

The careful reviewer will verify that the very verbose tracebacks are gone, that the UI is rendered as designed, and that this change doesn't break existing unit tests.

To post a comment you must log in.
Revision history for this message
Jonathan Rauprich (joni-noplu) wrote :

This patch fixes the problem on my machine (Ubuntu 10.04 x64), no problems noticed.

review: Approve
Revision history for this message
Florian Boucault (fboucault) wrote :

I dislike it because it is a local workaround. I would rather have a global fix. However the code is good to go.

It greatly improves the performance too.


review: Approve
Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks for the review! That's not my favourite solution either, but it does the job efficiently. The ideal solution would be to find a way to tell CCSUtils not to trim the units when the value is 0, but I don't think this is possible... I'm going to merge this one, and I'll be happy to review/merge a more global solution anyone may come up with.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'elisa-plugins/elisa/plugins/pigment/widgets/'
2--- elisa-plugins/elisa/plugins/pigment/widgets/ 2009-11-17 22:55:46 +0000
3+++ elisa-plugins/elisa/plugins/pigment/widgets/ 2010-05-29 18:24:33 +0000
4@@ -76,9 +76,21 @@
5 self._layout()
7 def update_style_properties(self, props=None):
8+ if props is None:
9+ return
10+ # Special properties: if they are updated, a re-layout is needed
11+ specials = ('top_height', 'bottom_height', 'left_width', 'right_width')
12+ needs_layout = False
13+ for key in specials:
14+ if key in props:
15+ needs_layout = True
16+ value = props[key]
17+ # The CSS parser may have trimed the unit if the value is '0'
18+ # (see
19+ if not isinstance(value, ValueWithUnit):
20+ props[key] = ValueWithUnit(value, 'px')
21 super(PiecePanel, self).update_style_properties(props)
22- if "top_height" in props or "bottom_height" in props or \
23- "left_width" in props or "right_width" in props:
24+ if needs_layout:
25 self._update_size()
26 self._layout()


People subscribed via source and target branches