Skip Menu |

This queue is for tickets about the CSS-Inliner CPAN distribution.

Report information
The Basics
Id: 85701
Status: resolved
Priority: 0/
Queue: CSS-Inliner

People
Owner: Nobody in particular
Requestors: spt [...] jobindex.dk
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 3674
Fixed in: (no value)



Subject: Implicitly universal pseudo-selectors handled incorrectly
CSS3 allows for pseudoselectors to be implicitly universal. That is, the selector ":focus" on its own is equivalent to writing "*:focus". See [1]. The regex on line 285 in CSS::Inliner r3674 [2] doesn't catch this, passing it on to HTML::Query, which proceeds to give a warning. Attached is a test-case illustrating the problem, and a patch that fixes it. [1]: http://www.w3.org/TR/selectors/#universal-selector [2]: https://metacpan.org/source/KAMELKEV/CSS-Inliner-3674/lib/CSS/Inliner.pm#L284
Subject: css-inliner-implicitly-universal.patch
diff --git lib/CSS/Inliner.pm lib/CSS/Inliner.pm index 1500b02..03ef17e 100644 --- lib/CSS/Inliner.pm +++ lib/CSS/Inliner.pm @@ -282,7 +282,7 @@ sub inlinify { my $properties = $$entry{properties}; #skip over psuedo selectors, they are not mappable the same - if ($selector =~ /[\w\*]:(?:(active|focus|hover|link|visited|after|before|selection|target|first-line|first-letter|first-child|first-child))\b/io) { + if ($selector =~ /(?:^|[\s\w\*]):(?:(active|focus|hover|link|visited|after|before|selection|target|first-line|first-letter|first-child|first-child))\b/io) { $self->_report_warning({ info => "The pseudo-class ':$1' cannot be supported inline" }); next; }
Subject: example.pl
#!/usr/bin/env perl use strict; use warnings; use CSS::Inliner; my $html = <<HTML; <html> <head> <style type="text/css"> :focus { } </style> </head> <body><span>Hello.</span></body> </html> HTML my $inliner = CSS::Inliner->new(); $inliner->read({ html => $html }); print $inliner->inlinify(); use Data::Dumper; print Dumper($inliner->content_warnings);
Hi, Sorry for the delay. I'm very interested in patches for this project. I will be looking into what you've provided here and will see if it's viable. Psuedo selector support is definitely weak right now. thanks, Kevin On Wed May 29 09:24:29 2013, spaaske wrote: Show quoted text
> CSS3 allows for pseudoselectors to be implicitly universal. That is, > the selector ":focus" on its own is equivalent to writing "*:focus". > See [1]. > > The regex on line 285 in CSS::Inliner r3674 [2] doesn't catch this, > passing it on to HTML::Query, which proceeds to give a warning. > > Attached is a test-case illustrating the problem, and a patch that > fixes it. > > [1]: http://www.w3.org/TR/selectors/#universal-selector > [2]: https://metacpan.org/source/KAMELKEV/CSS-Inliner- > 3674/lib/CSS/Inliner.pm#L284
There are additional problems here that exist despite the patch. For example a leading rule actually depends on whitespace to exist to be recognized by this rule - that's a bit of a problem. I'm working right now to try and understand exactly what cases you are attempting to resolve. I see your test, but it doesn't entirely explain what your intent is with restructuring the regex in this way. On Tue Jul 23 16:51:22 2013, KAMELKEV wrote: Show quoted text
> Hi, > > Sorry for the delay. I'm very interested in patches for this project. > I will be looking into what you've provided here and will see if it's > viable. > > Psuedo selector support is definitely weak right now. > > thanks, > Kevin > > On Wed May 29 09:24:29 2013, spaaske wrote:
> > CSS3 allows for pseudoselectors to be implicitly universal. That is, > > the selector ":focus" on its own is equivalent to writing "*:focus". > > See [1]. > > > > The regex on line 285 in CSS::Inliner r3674 [2] doesn't catch this, > > passing it on to HTML::Query, which proceeds to give a warning. > > > > Attached is a test-case illustrating the problem, and a patch that > > fixes it. > > > > [1]: http://www.w3.org/TR/selectors/#universal-selector > > [2]: https://metacpan.org/source/KAMELKEV/CSS-Inliner- > > 3674/lib/CSS/Inliner.pm#L284
Ok I get it. Previously we supported: *:focus div:focus but you are saying we now also need to support this case: :focus the proposed implementation explicitly requires either leading whitespace, a word character, or a start. It does not take into account the edge case whereby the rule is preceded by the style tag such as: <style>:focus{}</style> closer though - this is a good fix, I bet people are getting warnings like crazy On Fri Jul 26 20:41:49 2013, KAMELKEV wrote: Show quoted text
> There are additional problems here that exist despite the patch. For > example a leading rule actually depends on whitespace to exist to be > recognized by this rule - that's a bit of a problem. > > I'm working right now to try and understand exactly what cases you are > attempting to resolve. I see your test, but it doesn't entirely > explain what your intent is with restructuring the regex in this way. > > On Tue Jul 23 16:51:22 2013, KAMELKEV wrote:
> > Hi, > > > > Sorry for the delay. I'm very interested in patches for this project. > > I will be looking into what you've provided here and will see if it's > > viable. > > > > Psuedo selector support is definitely weak right now. > > > > thanks, > > Kevin > > > > On Wed May 29 09:24:29 2013, spaaske wrote:
> > > CSS3 allows for pseudoselectors to be implicitly universal. That > > > is, > > > the selector ":focus" on its own is equivalent to writing > > > "*:focus". > > > See [1]. > > > > > > The regex on line 285 in CSS::Inliner r3674 [2] doesn't catch this, > > > passing it on to HTML::Query, which proceeds to give a warning. > > > > > > Attached is a test-case illustrating the problem, and a patch that > > > fixes it. > > > > > > [1]: http://www.w3.org/TR/selectors/#universal-selector > > > [2]: https://metacpan.org/source/KAMELKEV/CSS-Inliner- > > > 3674/lib/CSS/Inliner.pm#L284
The idea here is committed to github here: https://github.com/kamelkev/CSS-Inliner/commit/2e364259ae08f9b99e32a9acb55083cd80697768 All selectors are parsed out and leading whitespace is thrown away, so the rule that looks at the selector doesn't need to look for whitespace in order to reject such a selector, so that part was removed from your regex. Additionally a test was added based on your test to ensure things were working as they should. I'm moving the entire repo to github, and all development will occur here in the future, so if you want to submit patches through there I can respond faster to you. I will be creating a cpan release shortly once I resolve the versioning issue that occurred when I moved from subversion to github. thanks, Kevin On Fri Jul 26 20:48:58 2013, KAMELKEV wrote: Show quoted text
> Ok I get it. Previously we supported: > > *:focus > div:focus > > but you are saying we now also need to support this case: > > :focus > > the proposed implementation explicitly requires either leading > whitespace, a word character, or a start. It does not take into > account the edge case whereby the rule is preceded by the style tag > such as: > > <style>:focus{}</style> > > closer though - this is a good fix, I bet people are getting warnings > like crazy > > > > > On Fri Jul 26 20:41:49 2013, KAMELKEV wrote:
> > There are additional problems here that exist despite the patch. For > > example a leading rule actually depends on whitespace to exist to be > > recognized by this rule - that's a bit of a problem. > > > > I'm working right now to try and understand exactly what cases you > > are > > attempting to resolve. I see your test, but it doesn't entirely > > explain what your intent is with restructuring the regex in this way. > > > > On Tue Jul 23 16:51:22 2013, KAMELKEV wrote:
> > > Hi, > > > > > > Sorry for the delay. I'm very interested in patches for this > > > project. > > > I will be looking into what you've provided here and will see if > > > it's > > > viable. > > > > > > Psuedo selector support is definitely weak right now. > > > > > > thanks, > > > Kevin > > > > > > On Wed May 29 09:24:29 2013, spaaske wrote:
> > > > CSS3 allows for pseudoselectors to be implicitly universal. That > > > > is, > > > > the selector ":focus" on its own is equivalent to writing > > > > "*:focus". > > > > See [1]. > > > > > > > > The regex on line 285 in CSS::Inliner r3674 [2] doesn't catch > > > > this, > > > > passing it on to HTML::Query, which proceeds to give a warning. > > > > > > > > Attached is a test-case illustrating the problem, and a patch > > > > that > > > > fixes it. > > > > > > > > [1]: http://www.w3.org/TR/selectors/#universal-selector > > > > [2]: https://metacpan.org/source/KAMELKEV/CSS-Inliner- > > > > 3674/lib/CSS/Inliner.pm#L284