Moved the escaping of the xpath locator to the NamedSelector

The NamedSelector has no valid reason to expect receiving an escaped value
for the XPath locator. Calling code has no reason to know that the locator
will be inserted in an XPath query.
The NamedSelector still support passing an escaped value for BC reasons
but this is deprecated.
The method SelectorsHandler::xpathLiteral and
Element::getSelectorsHandler have been deprecated as well.

Author: stof

driver-testsuite/tests/Form/SelectTest.php

@@ -66,8 +66,7 @@ public function testElementSelectedStateCheck($selectName, $optionValue, $option
         $session->visit($this->pathTo('/multiselect_form.html'));
         $select = $webAssert->fieldExists($selectName);
 
-        $optionValueEscaped = $session->getSelectorsHandler()->xpathLiteral($optionValue);
-        $option = $webAssert->elementExists('named', array('option', $optionValueEscaped));
+        $option = $webAssert->elementExists('named', array('option', $optionValue));
 
         $this->assertFalse($option->isSelected());
         $select->selectOption($optionText);

driver-testsuite/tests/TestCase.php

@@ -102,8 +102,6 @@ protected function getAssertSession()
      */
     protected function findById($id)
     {
-        $id = $this->getSession()->getSelectorsHandler()->xpathLiteral($id);
-
         return $this->getAssertSession()->elementExists('named', array('id', $id));
     }
 

src/Element/DocumentElement.php

@@ -46,8 +46,6 @@ public function getContent()
      */
     public function hasContent($content)
     {
-        return $this->has('named', array(
-            'content', $this->getSelectorsHandler()->xpathLiteral($content),
-        ));
+        return $this->has('named', array('content', $content));
     }
 }

src/Element/Element.php

@@ -85,6 +85,8 @@ protected function getDriver()
      * Returns selectors handler.
      *
      * @return SelectorsHandler
+     *
+     * @deprecated Accessing the selectors handler in the element is deprecated as of 1.7 and will be impossible in 2.0.
      */
     protected function getSelectorsHandler()
     {
@@ -156,7 +158,7 @@ public function findAll($selector, $locator)
             return $items;
         }
 
-        $xpath = $this->getSelectorsHandler()->selectorToXpath($selector, $locator);
+        $xpath = $this->selectorsHandler->selectorToXpath($selector, $locator);
         $xpath = $this->xpathManipulator->prepend($xpath, $this->getXpath());
 
         return $this->getDriver()->find($xpath);

src/Element/NodeElement.php

@@ -228,9 +228,7 @@ public function selectOption($option, $multiple = false)
             return;
         }
 
-        $opt = $this->find('named', array(
-            'option', $this->getSelectorsHandler()->xpathLiteral($option),
-        ));
+        $opt = $this->find('named', array('option', $option));
 
         if (null === $opt) {
             throw $this->elementNotFound('select option', 'value|text', $option);

src/Element/TraversableElement.php

@@ -28,8 +28,6 @@ abstract class TraversableElement extends Element
      */
     public function findById($id)
     {
-        $id = $this->getSelectorsHandler()->xpathLiteral($id);
-
         return $this->find('named', array('id', $id));
     }
 
@@ -54,9 +52,7 @@ public function hasLink($locator)
      */
     public function findLink($locator)
     {
-        return $this->find('named', array(
-            'link', $this->getSelectorsHandler()->xpathLiteral($locator),
-        ));
+        return $this->find('named', array('link', $locator));
     }
 
     /**
@@ -98,9 +94,7 @@ public function hasButton($locator)
      */
     public function findButton($locator)
     {
-        return $this->find('named', array(
-            'button', $this->getSelectorsHandler()->xpathLiteral($locator),
-        ));
+        return $this->find('named', array('button', $locator));
     }
 
     /**
@@ -142,9 +136,7 @@ public function hasField($locator)
      */
     public function findField($locator)
     {
-        return $this->find('named', array(
-            'field', $this->getSelectorsHandler()->xpathLiteral($locator),
-        ));
+        return $this->find('named', array('field', $locator));
     }
 
     /**
@@ -245,9 +237,7 @@ public function uncheckField($locator)
      */
     public function hasSelect($locator)
     {
-        return $this->has('named', array(
-            'select', $this->getSelectorsHandler()->xpathLiteral($locator),
-        ));
+        return $this->has('named', array('select', $locator));
     }
 
     /**
@@ -281,9 +271,7 @@ public function selectFieldOption($locator, $value, $multiple = false)
      */
     public function hasTable($locator)
     {
-        return $this->has('named', array(
-            'table', $this->getSelectorsHandler()->xpathLiteral($locator),
-        ));
+        return $this->has('named', array('table', $locator));
     }
 
     /**

src/Selector/NamedSelector.php

@@ -10,6 +10,8 @@
 
 namespace Behat\Mink\Selector;
 
+use Behat\Mink\Selector\Xpath\Escaper;
+
 /**
  * Named selectors engine. Uses registered XPath selectors to create new expressions.
  *
@@ -157,12 +159,15 @@ class NamedSelector implements SelectorInterface
 .//*[%idOrNameMatch%]
 XPATH
     );
+    private $xpathEscaper;
 
     /**
      * Creates selector instance.
      */
     public function __construct()
     {
+        $this->xpathEscaper = new Escaper();
+
         foreach ($this->replacements as $from => $to) {
             $this->replacements[$from] = strtr($to, $this->replacements);
         }
@@ -217,7 +222,7 @@ public function translateToXPath($locator)
         $xpath = $this->selectors[$selector];
 
         if (null !== $locator) {
-            $xpath = strtr($xpath, array('%locator%' => $locator));
+            $xpath = strtr($xpath, array('%locator%' => $this->escapeLocator($locator)));
         }
 
         return $xpath;
@@ -235,4 +240,24 @@ protected function registerReplacement($from, $to)
     {
         $this->replacements[$from] = $to;
     }
+
+    private function escapeLocator($locator)
+    {
+        // If the locator looks like an escaped one, don't escape it again for BC reasons.
+        if (
+            preg_match('/^\'[^\']*+\'$/', $locator)
+            || (false !== strpos($locator, '\'') && preg_match('/^"[^"]*+"$/', $locator))
+            || ((8 < $length = strlen($locator)) && 'concat(' === substr($locator, 0, 7) && ')' === $locator[$length - 1])
+        ) {
+            trigger_error(
+                'Passing an excaped locator to the named selector is deprecated as of 1.7 and will be removed in 2.0.'
+                .' Pass the raw value instead.',
+                E_USER_DEPRECATED
+            );
+
+            return $locator;
+        }
+
+        return $this->xpathEscaper->escapeLiteral($locator);
+    }
 }

src/Selector/SelectorsHandler.php

@@ -114,12 +114,22 @@ public function selectorToXpath($selector, $locator)
     /**
      * Translates string to XPath literal.
      *
+     * @deprecated since Mink 1.7. Use \Behat\Mink\Selector\Xpath\Escaper::escapeLiteral when building Xpath
+     *             or pass the unescaped value when using the named selector.
+     *
      * @param string $s
      *
      * @return string
      */
     public function xpathLiteral($s)
     {
+        trigger_error(
+            'The '.__METHOD__.' method is deprecated as of 1.7 and will be removed in 2.0.'
+            .' Use \Behat\Mink\Selector\Xpath\Escaper::escapeLiteral instead when building Xpath'
+            .' or pass the unescaped value when using the named selector.',
+            E_USER_DEPRECATED
+        );
+
         return $this->escaper->escapeLiteral($s);
     }
 }

tests/Element/ElementTest.php

@@ -36,11 +36,6 @@ protected function setUp()
 
         $this->selectors = $this->getMockBuilder('Behat\Mink\Selector\SelectorsHandler')->getMock();
         $this->session = new Session($this->driver, $this->selectors);
-
-        $this->selectors
-            ->expects($this->any())
-            ->method('xpathLiteral')
-            ->will($this->returnArgument(0));
     }
 
     protected function mockNamedFinder($xpath, array $results, $locator, $times = 2)

tests/Selector/NamedSelectorTest.php

@@ -3,7 +3,7 @@
 namespace Behat\Mink\Tests\Selector;
 
 use Behat\Mink\Selector\NamedSelector;
-use Behat\Mink\Selector\SelectorsHandler;
+use Behat\Mink\Selector\Xpath\Escaper;
 
 abstract class NamedSelectorTest extends \PHPUnit_Framework_TestCase
 {
@@ -42,10 +42,6 @@ public function testSelectors($fixtureFile, $selector, $locator, $expectedExactC
         $dom = new \DOMDocument('1.0', 'UTF-8');
         $dom->loadHTML(file_get_contents(__DIR__.'/fixtures/'.$fixtureFile));
 
-        // Escape the locator as Mink 1.x expects the caller of the NamedSelector to handle it
-        $selectorsHandler = new SelectorsHandler();
-        $locator = $selectorsHandler->xpathLiteral($locator);
-
         $namedSelector = $this->getSelector();
 
         $xpath = $namedSelector->translateToXPath(array($selector, $locator));
@@ -56,6 +52,20 @@ public function testSelectors($fixtureFile, $selector, $locator, $expectedExactC
         $this->assertEquals($expectedCount, $nodeList->length);
     }
 
+    /**
+     * @dataProvider getSelectorTests
+     */
+    public function testEscapedSelectors($fixtureFile, $selector, $locator, $expectedExactCount, $expectedPartialCount = null)
+    {
+        // Escape the locator as Mink 1.x expects the caller of the NamedSelector to handle it
+        $escaper = new Escaper();
+        $locator = $escaper->escapeLiteral($locator);
+
+        $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED);
+
+        $this->testSelectors($fixtureFile, $selector, $locator, $expectedExactCount, $expectedPartialCount);
+    }
+
     public function getSelectorTests()
     {
         $fieldCount = 8; // fields without `type` attribute
@@ -122,6 +132,7 @@ public function getSelectorTests()
 
             // 3 matches, because matches every HTML node in path: html > body > div
             'content' => array('test.html', 'content', 'content-text', 1, 4),
+            'content with quotes' => array('test.html', 'content', 'some "quoted" content', 1, 3),
 
             'select (name/label)' => array('test.html', 'select', 'the-field', 3),
             'select (with-id)' => array('test.html', 'select', 'the-field-select', 1),