From dd8595035fc122dc31f784fa75fccffb74256c57 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 4 Jul 2024 10:01:37 +0800 Subject: [PATCH] MDL-82373 behat: Stop calling ensure_node_is_visible before click W3C WebDriver Element::Click, Element::Clear, and Element::SendKeys all state that the WebDriver implementation (chromedriver, geckodriver, edgedriver) should scroll the element into view if it is not already visible. It is wrong for us to check if the element is visible or not before calling these as it may not be but will during the click/clear/type event. --- lib/behat/classes/behat_session_trait.php | 2 - lib/tests/behat/behat_action_menu.php | 2 - lib/tests/behat/behat_forms.php | 1 - lib/tests/behat/behat_general.php | 55 ++++++++++++++++--- mod/quiz/tests/behat/behat_mod_quiz.php | 1 - repository/tests/behat/behat_filepicker.php | 2 - .../tests/behat/behat_repository_upload.php | 1 - 7 files changed, 48 insertions(+), 16 deletions(-) diff --git a/lib/behat/classes/behat_session_trait.php b/lib/behat/classes/behat_session_trait.php index f6ef429d44f..0ec29e8dc91 100644 --- a/lib/behat/classes/behat_session_trait.php +++ b/lib/behat/classes/behat_session_trait.php @@ -645,7 +645,6 @@ trait behat_session_trait { * @return void Throws an exception if it times out without the element being visible */ protected function ensure_node_is_visible($node) { - if (!$this->running_javascript()) { return; } @@ -715,7 +714,6 @@ trait behat_session_trait { * @return NodeElement Throws an exception if it times out without being visible */ protected function ensure_element_is_visible($element, $selectortype) { - if (!$this->running_javascript()) { return; } diff --git a/lib/tests/behat/behat_action_menu.php b/lib/tests/behat/behat_action_menu.php index 6ec4631b74e..ba797f2e01f 100644 --- a/lib/tests/behat/behat_action_menu.php +++ b/lib/tests/behat/behat_action_menu.php @@ -62,7 +62,6 @@ class behat_action_menu extends behat_base { return; } - $this->ensure_node_is_visible($node); $node->click(); } @@ -80,7 +79,6 @@ class behat_action_menu extends behat_base { // Gets the node based on the requested selector type and locator. $menuselector = ".moodle-actionmenu .dropdown.show .dropdown-menu"; $node = $this->get_node_in_container("link", $menuitemstring, "css_element", $menuselector); - $this->ensure_node_is_visible($node); $node->click(); } diff --git a/lib/tests/behat/behat_forms.php b/lib/tests/behat/behat_forms.php index 7acbe001d40..fc12949cd6c 100644 --- a/lib/tests/behat/behat_forms.php +++ b/lib/tests/behat/behat_forms.php @@ -738,7 +738,6 @@ class behat_forms extends behat_base { public function i_expand_the_autocomplete($field) { $csstarget = '.form-autocomplete-downarrow'; $node = $this->get_node_in_container('css_element', $csstarget, 'form_row', $field); - $this->ensure_node_is_visible($node); $node->click(); } diff --git a/lib/tests/behat/behat_general.php b/lib/tests/behat/behat_general.php index 13127899142..863b1a65333 100644 --- a/lib/tests/behat/behat_general.php +++ b/lib/tests/behat/behat_general.php @@ -286,9 +286,7 @@ class behat_general extends behat_base { * @param string $link */ public function click_link($link) { - $linknode = $this->find_link($link); - $this->ensure_node_is_visible($linknode); $linknode->click(); } @@ -393,11 +391,8 @@ class behat_general extends behat_base { * @param string $selectortype The type of what we look for */ public function i_click_on($element, $selectortype) { - // Gets the node based on the requested selector type and locator. - $node = $this->get_selected_node($selectortype, $element); - $this->ensure_node_is_visible($node); - $node->click(); + $this->get_selected_node($selectortype, $element)->click(); } /** @@ -458,10 +453,56 @@ class behat_general extends behat_base { * @param string $nodeselectortype The type of selector where we look in */ public function i_click_on_in_the($element, $selectortype, $nodeelement, $nodeselectortype) { + $node = $this->get_node_in_container($selectortype, $element, $nodeselectortype, $nodeelement); + $node->click(); + } + + /** + * Click on the element with some modifier key pressed (alt, shift, meta or control). + * + * It is important to note that not all HTML elements are compatible with this step because + * the webdriver limitations. For example, alt click on checkboxes with a visible label will + * produce a normal checkbox click without the modifier. + * + * @When I :modifier click on :element :selectortype in the :nodeelement :nodeselectortype + * @param string $modifier the extra modifier to press (for example, alt+shift or shift) + * @param string $element Element we look for + * @param string $selectortype The type of what we look for + * @param string $nodeelement Element we look in + * @param string $nodeselectortype The type of selector where we look in + */ + public function i_key_click_on_in_the($modifier, $element, $selectortype, $nodeelement, $nodeselectortype) { + behat_base::require_javascript_in_session($this->getSession()); + + $key = null; + switch (strtoupper(trim($modifier))) { + case '': + break; + case 'SHIFT': + $key = behat_keys::SHIFT; + break; + case 'CTRL': + $key = behat_keys::CONTROL; + break; + case 'ALT': + $key = behat_keys::ALT; + break; + case 'META': + $key = behat_keys::META; + break; + default: + throw new \coding_exception("Unknown modifier key '$modifier'}"); + } $node = $this->get_node_in_container($selectortype, $element, $nodeselectortype, $nodeelement); - $this->ensure_node_is_visible($node); + + // KeyUP and KeyDown require the element to be displayed in the current window. + $this->execute_js_on_node($node, '{{ELEMENT}}.scrollIntoView();'); + $node->keyDown($key); $node->click(); + // Any click action can move the scroll. Ensure the element is still displayed. + $this->execute_js_on_node($node, '{{ELEMENT}}.scrollIntoView();'); + $node->keyUp($key); } /** diff --git a/mod/quiz/tests/behat/behat_mod_quiz.php b/mod/quiz/tests/behat/behat_mod_quiz.php index 996a9525382..ffc2a1714cd 100644 --- a/mod/quiz/tests/behat/behat_mod_quiz.php +++ b/mod/quiz/tests/behat/behat_mod_quiz.php @@ -579,7 +579,6 @@ class behat_mod_quiz extends behat_question_base { public function i_click_on_shuffle_for_section($heading) { $xpath = $this->get_xpath_for_shuffle_checkbox($heading); $checkbox = $this->find('xpath', $xpath); - $this->ensure_node_is_visible($checkbox); $checkbox->click(); } diff --git a/repository/tests/behat/behat_filepicker.php b/repository/tests/behat/behat_filepicker.php index ca8fffe514c..cce79ac5a3b 100644 --- a/repository/tests/behat/behat_filepicker.php +++ b/repository/tests/behat/behat_filepicker.php @@ -289,7 +289,6 @@ class behat_filepicker extends behat_base { } $selectfilebutton = $this->find_button(get_string('getfile', 'repository')); - $this->ensure_node_is_visible($selectfilebutton); $selectfilebutton->click(); // We wait for all the JS to finish as it is performing an action. @@ -297,7 +296,6 @@ class behat_filepicker extends behat_base { if ($overwriteaction !== false) { $overwritebutton = $this->find_button($overwriteaction); - $this->ensure_node_is_visible($overwritebutton); $overwritebutton->click(); // We wait for all the JS to finish. diff --git a/repository/upload/tests/behat/behat_repository_upload.php b/repository/upload/tests/behat/behat_repository_upload.php index 52fa09bbc0f..5f6a28ac00e 100644 --- a/repository/upload/tests/behat/behat_repository_upload.php +++ b/repository/upload/tests/behat/behat_repository_upload.php @@ -175,7 +175,6 @@ class behat_repository_upload extends behat_base { if ($overwriteaction !== false) { $overwritebutton = $this->find_button($overwriteaction); - $this->ensure_node_is_visible($overwritebutton); $overwritebutton->click(); // We wait for all the JS to finish.