Bug #59037 for CSS-Tiny: CSS::Tiny reorders rules

Thu Jul 01 17:51:40 2010 wonko [...] cpan.org - Ticket created

Subject:

CSS::Tiny reorders rules

The order of CSS is important in some circumstances and currently CSS::Tiny changes the order of the rules as it reads them in and then writes them out (in reverse alphabetical order). Order needs to be preserved or some styles could be broken. For instance, in this example: <html> <head> <style type="text/css"> .bar { color: blue } .foo { color: red } </style> <body> <h1 class="foo bar">Howdy!</h1> </body> </html> If you reverse the order of those rules you'll get different results. This is a simple contrived example, but I hope it conveys the point that order is important and if lost, then the stylesheet is not the same. I don't think this would be hard to fix and if you're willing to give me (Michael Peters) svn access I can make these changes.

Thu Jul 01 19:08:13 2010 wonko [...] cpan.org - Correspondence added

Looking at this again, it is kind of a tricky problem to solve. Since the CSS::Tiny object exposes it's internals as part of it's API and every key of the internal hash is a CSS rule, it means you can't put extra information in the object itself. So while I could keep track of what order things were read with some extra bookkeeping objects, to really be able to do this would require something like Tie::IxHash. I would settle for being able to use Tie::IxHash myself on the object after I've created it, but read() and read_string() can't be used on an existing object. Maybe that's what I should patch instead.

Thu Jul 01 19:08:14 2010 wonko [...] cpan.org - Status changed from 'new' to 'open'

Sat Jul 03 06:13:10 2010 adam [...] ali.as - Correspondence added

Subject:	Re: [rt.cpan.org #59037] CSS::Tiny reorders rules
Date:	Sat, 3 Jul 2010 20:12:59 +1000
To:	bug-CSS-Tiny [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

It's ::Tiny Dependencies are forbidden :) The loss of ordering in this case is intentional. People that need ordering support are encouraged to move up to CSS.pm, which supports a wider range of features. Adam K On 2 July 2010 09:08, Michael Peters via RT <bug-CSS-Tiny@rt.cpan.org> wrote: Show quoted text

> Queue: CSS-Tiny > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=59037 > > > Looking at this again, it is kind of a tricky problem to solve. Since > the CSS::Tiny object exposes it's internals as part of it's API and > every key of the internal hash is a CSS rule, it means you can't put > extra information in the object itself. So while I could keep track of > what order things were read with some extra bookkeeping objects, to > really be able to do this would require something like Tie::IxHash. > > I would settle for being able to use Tie::IxHash myself on the object > after I've created it, but read() and read_string() can't be used on an > existing object. Maybe that's what I should patch instead. >

Sat Jul 03 09:40:07 2010 wonko [...] cpan.org - Correspondence added

On Sat Jul 03 06:13:10 2010, adam@ali.as wrote: Show quoted text

> It's ::Tiny > > Dependencies are forbidden :)

Yeah that's why I didn't actually submit a patch for that. Show quoted text

> The loss of ordering in this case is intentional.

Really? Just seems like it's a bad idea to intentionally lose information when reading and then writing a CSS file. There isn't any other semantic information lost anywhere else (some syntactic information is lost, but that's minor and doesn't affect the correctness of the resulting CSS) so it seems strange that this information loss would be considered intentional. Show quoted text

> People that need ordering support are encouraged to move up to CSS.pm, > which supports a wider range of features.

It's not me that needs it per-se. If I was using CSS::Tiny directly then yes I could migrate to CSS.pm. But it's a dependency of another module that I'm using. But like I said, I couldn't see a way of adding this feature without adding a dependency or changing the API, both of which are non starters really. So I settled for my patch (https://rt.cpan.org/Ticket/Display.html?id=59039) that allows read() and read_string() to be used on an existing object. Now I just need to convince the author of this other module to tie his CSS::Tiny with Tie::IxHash before he reads. But, if you accept that patch would you be willing to accept a documentation patch noting how to preserve order of rules?

Sat Jul 03 11:34:14 2010 adam [...] ali.as - Correspondence added

Subject:	Re: [rt.cpan.org #59037] CSS::Tiny reorders rules
Date:	Sun, 4 Jul 2010 01:34:03 +1000
To:	bug-CSS-Tiny [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

If your patch can be done in only a couple of lines of code, yes. Adam K On 3 July 2010 23:40, Michael Peters via RT <bug-CSS-Tiny@rt.cpan.org> wrote: Show quoted text

> Queue: CSS-Tiny > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=59037 > > > On Sat Jul 03 06:13:10 2010, adam@ali.as wrote:

>> It's ::Tiny >> >> Dependencies are forbidden :)

> > Yeah that's why I didn't actually submit a patch for that. >

>> The loss of ordering in this case is intentional.

> > Really? Just seems like it's a bad idea to intentionally lose > information when reading and then writing a CSS file. There isn't any > other semantic information lost anywhere else (some syntactic > information is lost, but that's minor and doesn't affect the correctness > of the resulting CSS) so it seems strange that this information loss > would be considered intentional. >

>> People that need ordering support are encouraged to move up to CSS.pm, >> which supports a wider range of features.

> > It's not me that needs it per-se. If I was using CSS::Tiny directly then > yes I could migrate to CSS.pm. But it's a dependency of another module > that I'm using. > > But like I said, I couldn't see a way of adding this feature without > adding a dependency or changing the API, both of which are non starters > really. > > So I settled for my patch > (https://rt.cpan.org/Ticket/Display.html?id=59039) that allows read() > and read_string() to be used on an existing object. Now I just need to > convince the author of this other module to tie his CSS::Tiny with > Tie::IxHash before he reads. > > But, if you accept that patch would you be willing to accept a > documentation patch noting how to preserve order of rules? >

Fri Jul 09 16:11:40 2010 wonko [...] cpan.org - Correspondence added

On Sat Jul 03 11:34:14 2010, adam@ali.as wrote: Show quoted text

> If your patch can be done in only a couple of lines of code, yes.

Just in case it wasn't clear, I can't submit a documentation patch to explain how users can fix this themselves until this patch is also accepted: https://rt.cpan.org/Ticket/Display.html?id=59039 Basically you need to be able to do this: my $css = CSS::Tiny->new(); tie %$css, 'Tie::IxHash'; $css->read_string($self->{css}); But until RT #59039 is fixed, you can't call read_string() as an object method.

Sat Jul 10 21:59:13 2010 adam [...] ali.as - Correspondence added

Subject:	Re: [rt.cpan.org #59037] CSS::Tiny reorders rules
Date:	Sun, 11 Jul 2010 11:59:05 +1000
To:	bug-CSS-Tiny [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

It was fixed a week ago... http://cpansearch.perl.org/src/ADAMK/CSS-Tiny-1.17/Changes Adam K On 10 July 2010 06:11, Michael Peters via RT <bug-CSS-Tiny@rt.cpan.org> wrote: Show quoted text

> Queue: CSS-Tiny > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=59037 > > > On Sat Jul 03 11:34:14 2010, adam@ali.as wrote:

>> If your patch can be done in only a couple of lines of code, yes.

> > Just in case it wasn't clear, I can't submit a documentation patch to > explain how users can fix this themselves until this patch is also > accepted: https://rt.cpan.org/Ticket/Display.html?id=59039 > > Basically you need to be able to do this: > my $css = CSS::Tiny->new(); > tie %$css, 'Tie::IxHash'; > $css->read_string($self->{css}); > > But until RT #59039 is fixed, you can't call read_string() as an object > method. > > >

Mon Jul 12 13:46:40 2010 wonko [...] cpan.org - Correspondence added

On Sat Jul 10 21:59:13 2010, adam@ali.as wrote: Show quoted text

> It was fixed a week ago... > > http://cpansearch.perl.org/src/ADAMK/CSS-Tiny-1.17/Changes

Sorry, I didn't notice that. I was watching that bug ticket and since it didn't have any comments and is still marked as "open" that it was still unresolved. I've now closed that bug. So here's my patch that adds this small caveat.

Subject:

css_rule_order_doc.patch

Index: lib/CSS/Tiny.pm =================================================================== --- lib/CSS/Tiny.pm (revision 12620) +++ lib/CSS/Tiny.pm (working copy) @@ -284,6 +284,22 @@ When an error occurs, you can retrieve the error message either from the C<$CSS::Tiny::errstr> variable, or using the C<errstr> method. +=head1 CAVEATS + +=over + +=item CSS Rule Order + +While the order of rules in CSS is important, this is one of the features that +is sacrificed to keep things small and dependency-free. But if you need to preserve +order yourself, it can easily be done with the help of another module such as L<Tie::IxHash>: + + my $css = CSS::Tiny->new(); + tie %$css, 'Tie::IxHash'; # to preserve order of rules + $css->read('style.css'); + +=back + =head1 SUPPORT Bugs should be reported via the CPAN bug tracker at

Thu Sep 02 20:54:00 2010 adamk [...] cpan.org - Correspondence added

Documentation added in 1.19

Thu Sep 02 20:54:01 2010 adamk [...] cpan.org - Status changed from 'open' to 'resolved'