Skip to content

Commit

Permalink
Move declaration of replacements and selectors to new function
Browse files Browse the repository at this point in the history
At present the declaration of all raw un-processed selectors and their
replacements is in either the class body (private vars) or the
constructor.

This has the effect of modifying the original selectors on the class
instance vars during class instantiation, and also prevents any subclass
of ExactNamedSelector or PartialNamedSelector from making changes to any
replacement that has been explicitly overridden in those classes
constructors.

For example, it is impossible for a child class of either of these to
further override the '%tagTextMatch%' replacement because those changes
would either be overridden in the constructor of its parent, or all
original selector replacements would not be applied.

```
class MyPartialNamedSelector extends PartialNamedSelector
{

    public function __construct()
    {
        // Note: Calling the constructor here will mean that the call to
        // registerReplacement will update the replcaement, but it will
        // not have any effect upon any selector or replacement already
        // registered.
        parent::__construct();

        $this->registerReplacement('%tagTextMatch%', '%locator%');

        // Note: Calling the constructor here will cause our
        // tagTextMatch replacement to be overwritten by the one defined
        // in PartialNamedSelector.
        parent::__construct();
    }
}
```

Furthermore the original values of both the selectors and replacements
have been mutated in the constructor so it is not possible for any child
class to simply re-apply the original translations.

The only solution to this problem is to extend the NamedSelector base
class and manually copy the standard replacements from
PartialNamedSelector or ExactNamedSelector (as appropriate) into your
new child class, but this approach is not sustainable.

This patch modifies the Selector classes to:
* moves the storage and fetching of the original, unmodified and
  untranslated selectors and replacements to a function. This
  effectively makes them immutable; and
* allows the child classes to override or define their own selectors
  and replacements by simply overriding the parent class function and
  updating or adding the required array values.

This change allows the replacements (or selectors) to be updated as in
the following example:
```
class MyPartialNamedSelector extends PartialNamedSelector
{
    protected function getRawReplacements()
    {
        return array_merge(parent::getRawReplacements(), [
            '%tagTextMatch%' => '%locator%',
        ]);
    }

    protected function getRawSelectors()
    {
        return array_merge(parent::getRawSelectors(), [
            'link' => './/a[./@href][%tagTextMatch%]',
        ]);
    }
}
```

Note: For any usage where the child class was directly extending the
`NamedSelector` class and defining its custom selectors and replacements
in the constructor, these will need to be updated to define the relevant
`getRawSelectors()` and/or `getRawReplacements()` classes as above.

This change should be considered a possibly breaking change in this
situation and therefore would demand a new major release (1.10.0
presumably).

Whilst this is a breaking change, it will have provide a more
sustainable approach in future for more advanced uses of Mink.
  • Loading branch information
andrewnicols committed Oct 14, 2021
1 parent f7032c3 commit e128b99
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 80 deletions.
8 changes: 8 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
[Unreleased]
==================

* Move selector and replacement declaration to dedicated functions.
Note: If you were previously extending the NamedSelector class and defining overrides to existing values, these will
need to be moved to the relevant `getRawSelectors()` or `getRawReplacements()` function as appropriate.
See #815 for further information on this change.

1.9.0 / 2021-10-11
==================

Expand Down
18 changes: 9 additions & 9 deletions src/Selector/ExactNamedSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
*/
class ExactNamedSelector extends NamedSelector
{
public function __construct()
protected function getRawReplacements()
{
$this->registerReplacement('%tagTextMatch%', 'normalize-space(string(.)) = %locator%');
$this->registerReplacement('%valueMatch%', './@value = %locator%');
$this->registerReplacement('%titleMatch%', './@title = %locator%');
$this->registerReplacement('%altMatch%', './@alt = %locator%');
$this->registerReplacement('%relMatch%', './@rel = %locator%');
$this->registerReplacement('%labelAttributeMatch%', './@label = %locator%');

parent::__construct();
return array_merge(parent::getRawReplacements(), [
'%tagTextMatch%' => 'normalize-space(string(.)) = %locator%',
'%valueMatch%' => './@value = %locator%',
'%titleMatch%' => './@title = %locator%',
'%altMatch%' => './@alt = %locator%',
'%relMatch%' => './@rel = %locator%',
'%labelAttributeMatch%' => './@label = %locator%',
]);
}
}
146 changes: 84 additions & 62 deletions src/Selector/NamedSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,80 @@
*/
class NamedSelector implements SelectorInterface
{
private $replacements = array(
// simple replacements
'%lowercaseType%' => "translate(./@type, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz')",
'%lowercaseRole%' => "translate(./@role, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz')",
'%tagTextMatch%' => 'contains(normalize-space(string(.)), %locator%)',
'%labelTextMatch%' => './@id = //label[%tagTextMatch%]/@for',
'%idMatch%' => './@id = %locator%',
'%valueMatch%' => 'contains(./@value, %locator%)',
'%idOrValueMatch%' => '(%idMatch% or %valueMatch%)',
'%idOrNameMatch%' => '(%idMatch% or ./@name = %locator%)',
'%placeholderMatch%' => './@placeholder = %locator%',
'%titleMatch%' => 'contains(./@title, %locator%)',
'%altMatch%' => 'contains(./@alt, %locator%)',
'%relMatch%' => 'contains(./@rel, %locator%)',
'%labelAttributeMatch%' => 'contains(./@label, %locator%)',

// complex replacements
'%inputTypeWithoutPlaceholderFilter%' => "%lowercaseType% = 'radio' or %lowercaseType% = 'checkbox' or %lowercaseType% = 'file'",
'%fieldFilterWithPlaceholder%' => 'self::input[not(%inputTypeWithoutPlaceholderFilter%)] | self::textarea',
'%fieldMatchWithPlaceholder%' => '(%idOrNameMatch% or %labelTextMatch% or %placeholderMatch%)',
'%fieldMatchWithoutPlaceholder%' => '(%idOrNameMatch% or %labelTextMatch%)',
'%fieldFilterWithoutPlaceholder%' => 'self::input[%inputTypeWithoutPlaceholderFilter%] | self::select',
'%buttonTypeFilter%' => "%lowercaseType% = 'submit' or %lowercaseType% = 'image' or %lowercaseType% = 'button' or %lowercaseType% = 'reset'",
'%notFieldTypeFilter%' => "not(%buttonTypeFilter% or %lowercaseType% = 'hidden')",
'%buttonMatch%' => '%idOrNameMatch% or %valueMatch% or %titleMatch%',
'%linkMatch%' => '(%idMatch% or %tagTextMatch% or %titleMatch% or %relMatch%)',
'%imgAltMatch%' => './/img[%altMatch%]',
);

private $selectors = array(
'fieldset' => <<<XPATH
private $replacements = [];

private $selectors = [];

private $xpathEscaper;

/**
* Creates selector instance.
*/
public function __construct()
{
$this->xpathEscaper = new Escaper();

foreach ($this->getRawReplacements() as $from => $to) {
$this->registerReplacement($from, $to);
}

foreach ($this->getRawSelectors() as $alias => $selector) {
$this->registerNamedXpath($alias, $selector);
}
}

/**
* Get the list of replacements in their raw state with replacements not applied.
*
* @return array
*/
protected function getRawReplacements()
{
return array(
// simple replacements
'%lowercaseType%' => "translate(./@type, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz')",
'%lowercaseRole%' => "translate(./@role, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz')",
'%tagTextMatch%' => 'contains(normalize-space(string(.)), %locator%)',
'%labelTextMatch%' => './@id = //label[%tagTextMatch%]/@for',
'%idMatch%' => './@id = %locator%',
'%valueMatch%' => 'contains(./@value, %locator%)',
'%idOrValueMatch%' => '(%idMatch% or %valueMatch%)',
'%idOrNameMatch%' => '(%idMatch% or ./@name = %locator%)',
'%placeholderMatch%' => './@placeholder = %locator%',
'%titleMatch%' => 'contains(./@title, %locator%)',
'%altMatch%' => 'contains(./@alt, %locator%)',
'%relMatch%' => 'contains(./@rel, %locator%)',
'%labelAttributeMatch%' => 'contains(./@label, %locator%)',

// complex replacements
'%inputTypeWithoutPlaceholderFilter%' => "%lowercaseType% = 'radio' or %lowercaseType% = 'checkbox' or %lowercaseType% = 'file'",
'%fieldFilterWithPlaceholder%' => 'self::input[not(%inputTypeWithoutPlaceholderFilter%)] | self::textarea',
'%fieldMatchWithPlaceholder%' => '(%idOrNameMatch% or %labelTextMatch% or %placeholderMatch%)',
'%fieldMatchWithoutPlaceholder%' => '(%idOrNameMatch% or %labelTextMatch%)',
'%fieldFilterWithoutPlaceholder%' => 'self::input[%inputTypeWithoutPlaceholderFilter%] | self::select',
'%buttonTypeFilter%' => "%lowercaseType% = 'submit' or %lowercaseType% = 'image' or %lowercaseType% = 'button' or %lowercaseType% = 'reset'",
'%notFieldTypeFilter%' => "not(%buttonTypeFilter% or %lowercaseType% = 'hidden')",
'%buttonMatch%' => '%idOrNameMatch% or %valueMatch% or %titleMatch%',
'%linkMatch%' => '(%idMatch% or %tagTextMatch% or %titleMatch% or %relMatch%)',
'%imgAltMatch%' => './/img[%altMatch%]',
);
}

/**
* Get the list of selectors in their raw state with replacements not applied.
*
* @return array
*/
protected function getRawSelectors()
{

return array(
'fieldset' => <<<XPATH
.//fieldset
[(%idMatch% or .//legend[%tagTextMatch%])]
XPATH

,'field' => <<<XPATH
,'field' => <<<XPATH
.//*
[%fieldFilterWithPlaceholder%][%notFieldTypeFilter%][%fieldMatchWithPlaceholder%]
|
Expand All @@ -66,15 +104,15 @@ class NamedSelector implements SelectorInterface
.//label[%tagTextMatch%]//.//*[%fieldFilterWithoutPlaceholder%][%notFieldTypeFilter%]
XPATH

,'link' => <<<XPATH
,'link' => <<<XPATH
.//a
[./@href][(%linkMatch% or %imgAltMatch%)]
|
.//*
[%lowercaseRole% = 'link'][(%idOrValueMatch% or %titleMatch% or %tagTextMatch%)]
XPATH

,'button' => <<<XPATH
,'button' => <<<XPATH
.//input
[%buttonTypeFilter%][(%buttonMatch%)]
|
Expand All @@ -88,7 +126,7 @@ class NamedSelector implements SelectorInterface
[%lowercaseRole% = 'button'][(%buttonMatch% or %tagTextMatch%)]
XPATH

,'link_or_button' => <<<XPATH
,'link_or_button' => <<<XPATH
.//a
[./@href][(%linkMatch% or %imgAltMatch%)]
|
Expand All @@ -105,76 +143,60 @@ class NamedSelector implements SelectorInterface
[(%lowercaseRole% = 'button' or %lowercaseRole% = 'link')][(%idOrValueMatch% or %titleMatch% or %tagTextMatch%)]
XPATH

,'content' => <<<XPATH
,'content' => <<<XPATH
./descendant-or-self::*
[%tagTextMatch%]
XPATH

,'select' => <<<XPATH
,'select' => <<<XPATH
.//select
[%fieldMatchWithoutPlaceholder%]
|
.//label[%tagTextMatch%]//.//select
XPATH

,'checkbox' => <<<XPATH
,'checkbox' => <<<XPATH
.//input
[%lowercaseType% = 'checkbox'][%fieldMatchWithoutPlaceholder%]
|
.//label[%tagTextMatch%]//.//input[%lowercaseType% = 'checkbox']
XPATH

,'radio' => <<<XPATH
,'radio' => <<<XPATH
.//input
[%lowercaseType% = 'radio'][%fieldMatchWithoutPlaceholder%]
|
.//label[%tagTextMatch%]//.//input[%lowercaseType% = 'radio']
XPATH

,'file' => <<<XPATH
,'file' => <<<XPATH
.//input
[%lowercaseType% = 'file'][%fieldMatchWithoutPlaceholder%]
|
.//label[%tagTextMatch%]//.//input[%lowercaseType% = 'file']
XPATH

,'optgroup' => <<<XPATH
,'optgroup' => <<<XPATH
.//optgroup
[%labelAttributeMatch%]
XPATH

,'option' => <<<XPATH
,'option' => <<<XPATH
.//option
[(./@value = %locator% or %tagTextMatch%)]
XPATH

,'table' => <<<XPATH
,'table' => <<<XPATH
.//table
[(%idMatch% or .//caption[%tagTextMatch%])]
XPATH
,'id' => <<<XPATH
,'id' => <<<XPATH
.//*[%idMatch%]
XPATH
,'id_or_name' => <<<XPATH
,'id_or_name' => <<<XPATH
.//*[%idOrNameMatch%]
XPATH
);
private $xpathEscaper;

/**
* Creates selector instance.
*/
public function __construct()
{
$this->xpathEscaper = new Escaper();

foreach ($this->replacements as $from => $to) {
$this->registerReplacement($from, $to);
}

foreach ($this->selectors as $alias => $selector) {
$this->registerNamedXpath($alias, $selector);
}
);
}

/**
Expand Down
18 changes: 9 additions & 9 deletions src/Selector/PartialNamedSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@
*/
class PartialNamedSelector extends NamedSelector
{
public function __construct()
protected function getRawReplacements()
{
$this->registerReplacement('%tagTextMatch%', 'contains(normalize-space(string(.)), %locator%)');
$this->registerReplacement('%valueMatch%', 'contains(./@value, %locator%)');
$this->registerReplacement('%titleMatch%', 'contains(./@title, %locator%)');
$this->registerReplacement('%altMatch%', 'contains(./@alt, %locator%)');
$this->registerReplacement('%relMatch%', 'contains(./@rel, %locator%)');
$this->registerReplacement('%labelAttributeMatch%', 'contains(./@label, %locator%)');

parent::__construct();
return array_merge(parent::getRawReplacements(), [
'%tagTextMatch%' => 'contains(normalize-space(string(.)), %locator%)',
'%valueMatch%' => 'contains(./@value, %locator%)',
'%titleMatch%' => 'contains(./@title, %locator%)',
'%altMatch%' => 'contains(./@alt, %locator%)',
'%relMatch%' => 'contains(./@rel, %locator%)',
'%labelAttributeMatch%' => 'contains(./@label, %locator%)',
]);
}
}

0 comments on commit e128b99

Please sign in to comment.