Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fixes #5310 #5351

Closed
wants to merge 3 commits into from

3 participants

@steverhoades

I was incorrect in my initial review of 5310, this class is meant to do more than just css selectors.

@Maks3w Maks3w commented on the diff
tests/ZendTest/Dom/Css2XpathTest.php
@@ -164,4 +164,16 @@ public function testIdSelectorWithLeadingAsterix()
$test = Css2Xpath::transform('*#id');
$this->assertEquals("//*[@id='id']", $test);
}
+
+ /**
+ * @group ZF-5310
+ */
+ public function testCanTransformWithAttributeAndDot()
+ {
+ $test = Css2Xpath::transform('a[href="http://example.com"]');
+ $this->assertEquals("//a[@href='http://example.com']", $test);
+
+ $test = Css2Xpath::transform('a[@href="http://example.com"]');
@Maks3w Collaborator
Maks3w added a note

Should this throw a exception since @ is not a valid CSS selector?

@Maks3w Thats a great question, the fact is, the function converts the code to include the @ symbol, I think it would be weird to throw an exception if one includes the @ symbol, seeing as the result of the function outputs the correctly formatted query.

EDIT: to clarify above

$test = Css2Xpath::transform('a[href="http://example.com"]');
echo $test; // outputs: //a[@href='http://example.com']

The reason I included that test was to ensure that when an @ was included that the double quotes were converted to a single quote, so the 2 were consistent. It does seem odd to me that the CSS2XPath is modifying this query in general, and perhaps that is the modification that needs to be made in this regard, IE> ignore attributes? Unfortunately I don't know enough of the history of this class to make that decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@steverhoades

I've added an additional test to QueryTest, I would imagine that this is the preferred result from the Dom/Query class.

@weierophinney weierophinney closed this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/5351'
Close #5351
Fixes #5310
1ca40da
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 25, 2013
  1. @steverhoades

    fixes 5310

    steverhoades authored
Commits on Oct 26, 2013
  1. @steverhoades
Commits on Oct 27, 2013
  1. @steverhoades
This page is out of date. Refresh to see the latest.
View
14 library/Zend/Dom/Css2Xpath.php
@@ -85,7 +85,7 @@ protected static function _tokenize($expression)
// arbitrary attribute strict equality
$expression = preg_replace_callback(
- '|\[([a-z0-9_-]+)=[\'"]([^\'"]+)[\'"]\]|i',
+ '|\[@?([a-z0-9_-]+)=[\'"]([^\'"]+)[\'"]\]|i',
function ($matches) {
return '[@' . strtolower($matches[1]) . "='" . $matches[2] . "']";
},
@@ -113,11 +113,13 @@ function ($matches) {
);
// Classes
- $expression = preg_replace(
- '|\.([a-z][a-z0-9_-]*)|i',
- "[contains(concat(' ', normalize-space(@class), ' '), ' \$1 ')]",
- $expression
- );
+ if(false === strpos($expression, "[@")) {
+ $expression = preg_replace(
+ '|\.([a-z][a-z0-9_-]*)|i',
+ "[contains(concat(' ', normalize-space(@class), ' '), ' \$1 ')]",
+ $expression
+ );
+ }
/** ZF-9764 -- remove double asterisk */
$expression = str_replace('**', '*', $expression);
View
12 tests/ZendTest/Dom/Css2XpathTest.php
@@ -164,4 +164,16 @@ public function testIdSelectorWithLeadingAsterix()
$test = Css2Xpath::transform('*#id');
$this->assertEquals("//*[@id='id']", $test);
}
+
+ /**
+ * @group ZF-5310
+ */
+ public function testCanTransformWithAttributeAndDot()
+ {
+ $test = Css2Xpath::transform('a[href="http://example.com"]');
+ $this->assertEquals("//a[@href='http://example.com']", $test);
+
+ $test = Css2Xpath::transform('a[@href="http://example.com"]');
@Maks3w Collaborator
Maks3w added a note

Should this throw a exception since @ is not a valid CSS selector?

@Maks3w Thats a great question, the fact is, the function converts the code to include the @ symbol, I think it would be weird to throw an exception if one includes the @ symbol, seeing as the result of the function outputs the correctly formatted query.

EDIT: to clarify above

$test = Css2Xpath::transform('a[href="http://example.com"]');
echo $test; // outputs: //a[@href='http://example.com']

The reason I included that test was to ensure that when an @ was included that the double quotes were converted to a single quote, so the 2 were consistent. It does seem odd to me that the CSS2XPath is modifying this query in general, and perhaps that is the modification that needs to be made in this regard, IE> ignore attributes? Unfortunately I don't know enough of the history of this class to make that decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $this->assertEquals("//a[@href='http://example.com']", $test);
+ }
}
View
13 tests/ZendTest/Dom/QueryTest.php
@@ -396,4 +396,17 @@ public function testOffsetUnset()
unset($results[2]);
}
+
+ /**
+ * @group ZF-5310
+ */
+ public function testCssSelectorShouldFindNodesWhenMatchingAttributeValueWithDot()
+ {
+ $this->loadHtml();
+ $results = $this->query->execute('a[href="http://www.about.com"]');
+
+ $this->assertEquals(1, $results->count());
+ $this->assertEquals('About', $results[0]->nodeValue);
+
+ }
}
View
2  tests/ZendTest/Dom/_files/sample.xhtml
@@ -16,7 +16,7 @@
<ul id="topnav">
<li class="current"><a href="#">Home</a></li>
- <li><a href="#">About</a></li>
+ <li><a href="http://www.about.com">About</a></li>
<li><a href="#">Misc</a></li>
<li><a href="#">Wiki</a></li>
</ul>
Something went wrong with that request. Please try again.