From e128b995146068f192b92df6b607a973a172467c Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 14 Oct 2021 07:21:27 +0800 Subject: [PATCH] Move declaration of replacements and selectors to new function 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. --- CHANGES.md | 8 ++ src/Selector/ExactNamedSelector.php | 18 ++-- src/Selector/NamedSelector.php | 146 +++++++++++++++----------- src/Selector/PartialNamedSelector.php | 18 ++-- 4 files changed, 110 insertions(+), 80 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index b40f35551..1829dd9e3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 ================== diff --git a/src/Selector/ExactNamedSelector.php b/src/Selector/ExactNamedSelector.php index 03315a9cf..8b86a2acc 100644 --- a/src/Selector/ExactNamedSelector.php +++ b/src/Selector/ExactNamedSelector.php @@ -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%', + ]); } } diff --git a/src/Selector/NamedSelector.php b/src/Selector/NamedSelector.php index 12560b18a..fb00ae28e 100644 --- a/src/Selector/NamedSelector.php +++ b/src/Selector/NamedSelector.php @@ -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' => <<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' => << << << << << << << << << << << << << << << << << << << << << << << << << << << << <<xpathEscaper = new Escaper(); - - foreach ($this->replacements as $from => $to) { - $this->registerReplacement($from, $to); - } - - foreach ($this->selectors as $alias => $selector) { - $this->registerNamedXpath($alias, $selector); - } + ); } /** diff --git a/src/Selector/PartialNamedSelector.php b/src/Selector/PartialNamedSelector.php index b864a2265..d5faed769 100644 --- a/src/Selector/PartialNamedSelector.php +++ b/src/Selector/PartialNamedSelector.php @@ -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%)', + ]); } }