fix a few bugs in Mojo::DOM::CSS that required class, id and attribut…

…e selectors, as well as pseudo-classes, to be in a specific order
mojolicious · Mar 20, 2015 · fc52a98 · fc52a98
1 parent 55ba812
commit fc52a98
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 52 deletions.
diff --git a/Changes b/Changes
@@ -1,7 +1,7 @@
 
 6.04  2015-03-20
-  - Fixed bug in Mojo::DOM::CSS where pseudo-classes following attribute
-    selectors were not handled correctly.
+  - Fixed a few bugs in Mojo::DOM::CSS that required class, id and attribute
+    selectors, as well as pseudo-classes, to be in a specific order.
 
 6.03  2015-03-16
   - Added support for overriding the HTTP request method with the _method query

diff --git a/lib/Mojo/DOM/CSS.pm b/lib/Mojo/DOM/CSS.pm
@@ -14,15 +14,7 @@ my $ATTR_RE   = qr/
   )?
   \]
 /x;
-my $PSEUDO_CLASS_RE = qr/(?::([\w\-]+)(?:\(((?:\([^)]+\)|[^)])+)\))?)/;
-my $TOKEN_RE        = qr/
-  (\s*,\s*)?                            # Separator
-  ((?:[^[\\:\s,>+~]|$ESCAPE_RE\s?)+)?   # Element
-  ($PSEUDO_CLASS_RE*)?                  # Pseudo-class
-  ((?:$ATTR_RE)*)?                      # Attributes
-  ($PSEUDO_CLASS_RE*)?                  # Pseudo-class (again)
-  (?:\s*([>+~]))?                       # Combinator
-/x;
+my $PSEUDO_CLASS_RE = qr/:([\w\-]+)(?:\(((?:\([^)]+\)|[^)])+)\))?/;
 
 sub matches {
   my $tree = shift->tree;
@@ -81,41 +73,41 @@ sub _compile {
   my $css = "$_[0]";
 
   my $pattern = [[]];
-  while ($css =~ /$TOKEN_RE/go) {
-    my ($separator, $element, $pc, $attrs, $pc2, $combinator)
-      = ($1, $2 // '', $3, $6, $12, $15);
-
-    next unless $separator || $element || $pc || $attrs || $pc2 || $combinator;
+  while ($css) {
 
-    # New selector
-    push @$pattern, [] if $separator;
+    # Separator
     my $part = $pattern->[-1];
+    push @$part, [] unless @$part && ref $part->[-1];
+    my $selector = $part->[-1];
+    if ($css =~ s/^\s*,\s*//) { push @$pattern, [] }
 
-    # Empty combinator
-    push @$part, ' ' if $part->[-1] && ref $part->[-1];
-
-    # Tag
-    push @$part, my $selector = [];
-    push @$selector, ['tag', _name($1)]
-      if $element =~ s/^((?:\\\.|\\\#|[^.#])+)// && $1 ne '*';
+    # Combinator
+    elsif ($css =~ s/^\s*([ >+~])\s*//) { push @$part, $1 }
 
     # Class or ID
-    while ($element =~ /(?:([.#])((?:\\[.\#]|[^\#.])+))/g) {
+    elsif ($css =~ s/^([.#])((?:$ESCAPE_RE\s|\\.|[^,.#:[ >~+])+)//o) {
       my ($name, $op) = $1 eq '.' ? ('class', '~') : ('id', '');
       push @$selector, ['attr', _name($name), _value($op, $2)];
     }
 
-    # Pseudo-classes (":not" contains more selectors)
-    $pc = $pc && $pc2 ? "$pc$pc2" : $pc || $pc2;
-    push @$selector, ['pc', lc $1, $1 eq 'not' ? _compile($2) : _equation($2)]
-      while $pc =~ /$PSEUDO_CLASS_RE/go;
-
     # Attributes
-    push @$selector, ['attr', _name($1), _value($2 // '', $3 // $4, $5)]
-      while $attrs =~ /$ATTR_RE/go;
+    elsif ($css =~ s/^$ATTR_RE//o) {
+      push @$selector, ['attr', _name($1), _value($2 // '', $3 // $4, $5)];
+    }
+
+    # Pseudo-class (":not" contains more selectors)
+    elsif ($css =~ s/^$PSEUDO_CLASS_RE//o) {
+      push @$selector,
+        ['pc', lc $1, $1 eq 'not' ? _compile($2) : _equation($2)];
+    }
+
+    # Tag
+    elsif ($css =~ s/^((?:$ESCAPE_RE\s|\\.|[^,.#:[ >~+])+)//o) {
+      push @$selector, ['tag', _name($1)] unless $1 eq '*';
+    }
+
+    else {last}
 
-    # Combinator
-    push @$part, $combinator if $combinator;
   }
 
   return $pattern;

diff --git a/t/mojo/dom.t b/t/mojo/dom.t
@@ -808,7 +808,10 @@ is $dom->content, '<a>xxx<x>x</x>xxx</a>', 'right result';
 
 # Multiple selectors
 $dom = Mojo::DOM->new(
-  '<div id="a">A</div><div id="b">B</div><div id="c">C</div>');
+  '<div id="a">A</div><div id="b">B</div><div id="c">C</div><p>D</p>');
+@div = ();
+$dom->find('p, div')->each(sub { push @div, shift->text });
+is_deeply \@div, [qw(A B C D)], 'found all elements';
 @div = ();
 $dom->find('#a, #c')->each(sub { push @div, shift->text });
 is_deeply \@div, [qw(A C)], 'found all div elements with the right ids';
@@ -1161,19 +1164,6 @@ is_deeply \@e, ['J'], 'found only child';
 $dom->find('div div:only-of-type')->each(sub { push @e, shift->text });
 is_deeply \@e, [qw(J K)], 'found only child';
 
-# Pseudo-classes in different places
-$dom = Mojo::DOM->new('<a href="http://foo">It works!</a>');
-is $dom->at('a:not([href*="example.com"])[href^="http"]')->text, 'It works!',
-  'right text';
-is $dom->at(':not([href*="foo"])a[href^="http"]'), undef, 'no result';
-is $dom->at('a[href^="http"]:not([href*="example.com"])')->text, 'It works!',
-  'right text';
-is $dom->at('a[href^="http"]:not([href*="foo"])'), undef, 'no result';
-is $dom->at('a:not(b)[href^="h"]:not([href*="e"])')->text, 'It works!',
-  'right text';
-is $dom->at('a:not(a)[href^="h"]:not([href*="e"])'), undef, 'no result';
-is $dom->at('a:not(b)[href^="h"]:not([href*="f"])'), undef, 'no result';
-
 # Sibling combinator
 $dom = Mojo::DOM->new(<<EOF);
 <ul>
@@ -1183,7 +1173,7 @@ $dom = Mojo::DOM->new(<<EOF);
 </ul>
 <h1>D</h1>
 <p id="♥">E</p>
-<p id="☃">F</p>
+<p id="☃">F<b>H</b></p>
 <div>G</div>
 EOF
 is $dom->at('li ~ p')->text,       'B', 'right text';
@@ -1207,8 +1197,9 @@ is $dom->at('ul + h1 + p + p + div')->text, 'G', 'right text';
 is $dom->at('ul + h1 ~ p + div')->text,     'G', 'right text';
 is $dom->at('h1 ~ #♥')->text,             'E', 'right text';
 is $dom->at('h1 + #♥')->text,             'E', 'right text';
-is $dom->at('#♥ ~ #☃')->text,           'F', 'right text';
-is $dom->at('#♥ + #☃')->text,           'F', 'right text';
+is $dom->at('#♥~#☃')->text,             'F', 'right text';
+is $dom->at('#♥+#☃')->text,             'F', 'right text';
+is $dom->at('#♥+#☃>b')->text,           'H', 'right text';
 is $dom->at('#♥ > #☃'), undef, 'no result';
 is $dom->at('#♥ #☃'),   undef, 'no result';
 is $dom->at('#♥ + #☃ + :nth-last-child(1)')->text,  'G', 'right text';
@@ -2370,6 +2361,20 @@ is $dom->find('div > ul li')->[2], undef, 'no result';
 is $dom->find('div > ul ul')->[0]->text, 'C', 'right text';
 is $dom->find('div > ul ul')->[1], undef, 'no result';
 
+# Unusual order
+$dom = Mojo::DOM->new(
+  '<a href="http://example.com" id="foo" class="bar">Ok!</a>');
+is $dom->at('a:not([href$=foo])[href^=h]')->text, 'Ok!', 'right text';
+is $dom->at('a:not([href$=example.com])[href^=h]'), undef, 'no result';
+is $dom->at('a[href^=h]#foo.bar')->text, 'Ok!', 'right text';
+is $dom->at('a[href^=h]#foo.baz'), undef, 'no result';
+is $dom->at('a[href^=h]#foo:not(b)')->text, 'Ok!', 'right text';
+is $dom->at('a[href^=h]#foo:not(a)'), undef, 'no result';
+is $dom->at('[href^=h].bar:not(b)[href$=m]#foo')->text, 'Ok!', 'right text';
+is $dom->at('[href^=h].bar:not(b)[href$=m]#bar'), undef, 'no result';
+is $dom->at(':not(b)#foo#foo')->text, 'Ok!', 'right text';
+is $dom->at(':not(b)#foo#bar'), undef, 'no result';
+
 # Slash between attributes
 $dom = Mojo::DOM->new('<input /type=checkbox / value="/a/" checked/><br/>');
 is_deeply $dom->at('input')->attr,