Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fixes #5310 #5351

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

steverhoades commented Oct 25, 2013

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 Oct 26, 2013

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

Maks3w Oct 26, 2013

Member

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

@steverhoades

steverhoades Oct 26, 2013

Contributor

@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.

Contributor

steverhoades commented Oct 26, 2013

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

@ghost ghost assigned weierophinney Oct 28, 2013

weierophinney added a commit that referenced this pull request Oct 28, 2013

weierophinney added a commit that referenced this pull request Oct 28, 2013

weierophinney added a commit to zendframework/zend-dom that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-dom that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-dom that referenced this pull request May 15, 2015

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