From 6d36c612ff39c3e942d221011a508f2835a62293 Mon Sep 17 00:00:00 2001 From: Victor Grousset/tuxayo Date: Thu, 19 Nov 2020 13:47:45 +0100 Subject: [PATCH] Bug 27055: Fix compatibility with newer Firefox+Selenium version Fix "submit is not a function error" A submit button should not be named "submit", in this case, it's id. https://stackoverflow.com/questions/833032/submit-is-not-a-function-error-in-javascript Fix some uses of get_attribute() Fix a fail by setting a global implicit_wait_timeout, default value is 0 in our lib. Other libs set it higher which helps to not have to manually deal with part of the timing issues. Fix: remove usage of click_when_visible() because it doesn't work with elements not in the top of the page. Because they are off screen. Fix: use $driver->quit() in error_handler to not forget an open Firefox. With the current version, it fills /dev/shm and fails with around 5 Firefox opened. Also use quit() it at the end of every script. Fix: filling item fields, to fill only the displayed one (not those with display:none) == Test plan == 1. Update selenium/standalone-firefox to the latest version [1] 2. prove t/db_dependent/selenium/authentication.t 3. It fails with: arguments[0].form.submit is not a function 4. Apply patch 5. Retest 6. Success [1] In koha-testing-docker you can do it with docker-compose.yml: selenium: - image: selenium/standalone-firefox:2.53.1-americium + image: selenium/standalone-firefox Signed-off-by: Mason James Signed-off-by: Tomas Cohen Arazi --- koha-tmpl/intranet-tmpl/prog/css/login.css | 2 +- koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt | 2 +- t/db_dependent/selenium/basic_workflow.t | 7 +++---- t/db_dependent/selenium/patrons_search.t | 2 ++ t/db_dependent/selenium/regressions.t | 12 ++++++++---- t/db_dependent/selenium/update_child_to_adult.t | 5 ++++- t/lib/Selenium.pm | 11 ++++++----- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/css/login.css b/koha-tmpl/intranet-tmpl/prog/css/login.css index f4d0fb166b..722e9f420c 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/login.css +++ b/koha-tmpl/intranet-tmpl/prog/css/login.css @@ -121,7 +121,7 @@ label { width : 99%; } -#login #submit, #login .button { +#login #submit-button, #login .button { font-size: 1.4em; padding : .3em .6em; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt index b980f1e3af..046f288bcf 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -112,7 +112,7 @@ -

+

[% IF ( casAuthentication ) %]

Cas login

diff --git a/t/db_dependent/selenium/basic_workflow.t b/t/db_dependent/selenium/basic_workflow.t index c4bb0310c5..4c570612a2 100755 --- a/t/db_dependent/selenium/basic_workflow.t +++ b/t/db_dependent/selenium/basic_workflow.t @@ -177,10 +177,9 @@ SKIP: { $driver->get($base_url."/cataloguing/additem.pl?biblionumber=$biblionumber"); like( $driver->get_title(), qr(test biblio \d+ by test author), ); my $form = $driver->find_element('//form[@name="f"]'); - my $inputs = $driver->find_child_elements($form, '//input[@type="text"]'); + # select the text inputs that don't have display:none + my $inputs = $driver->find_child_elements($form, '/.//*[not(self::node()[contains(@style,"display:none")])]/*[@type="text"]'); for my $input ( @$inputs ) { - next if $input->is_hidden(); - my $id = $input->get_attribute('id'); next unless $id =~ m|^tag_952_subfield|; @@ -206,7 +205,7 @@ SKIP: { elsif ( $id =~ m|^tag_952_subfield_d| # dateaccessioned ) { - $v = ""; # The input has been prefilled with %Y-%m-%d already + next; # The input has been prefilled with %Y-%m-%d already } elsif ( $id =~ m|^tag_952_subfield_3| # materials diff --git a/t/db_dependent/selenium/patrons_search.t b/t/db_dependent/selenium/patrons_search.t index 5ee367a1e4..46920e8e1c 100755 --- a/t/db_dependent/selenium/patrons_search.t +++ b/t/db_dependent/selenium/patrons_search.t @@ -134,6 +134,8 @@ subtest 'Search patrons' => sub { push @cleanup, $library; push @cleanup, $patron_category; C4::Context->set_preference('DefaultPatronSearchFields',$default_patron_search_fields); + + $driver->quit(); }; END { diff --git a/t/db_dependent/selenium/regressions.t b/t/db_dependent/selenium/regressions.t index c18c95a837..c6d254eb74 100755 --- a/t/db_dependent/selenium/regressions.t +++ b/t/db_dependent/selenium/regressions.t @@ -59,10 +59,12 @@ subtest 'OPAC - borrowernumber and branchcode as html attributes' => sub { $patron->set_password({ password => $password }); $s->opac_auth( $patron->userid, $password ); my $elt = $driver->find_element('//span[@class="loggedinusername"]'); - is( $elt->get_attribute('data-branchcode'), $patron->library->branchcode, + is( $elt->get_attribute('data-branchcode', 1), $patron->library->branchcode, "Since bug 20921 span.loggedinusername should contain data-branchcode" + # No idea why we need the second param of get_attribute(). As + # data-branchcode is still there after page finished loading. ); - is( $elt->get_attribute('data-borrowernumber'), $patron->borrowernumber, + is( $elt->get_attribute('data-borrowernumber', 1), $patron->borrowernumber, "Since bug 20921 span.loggedinusername should contain data-borrowernumber" ); push @cleanup, $patron, $patron->category, $patron->library; @@ -168,11 +170,11 @@ subtest 'Display circulation table correctly' => sub { my @thead_th = $driver->find_elements('//table[@id="issues-table"]/thead/tr/th'); my $thead_length = 0; - $thead_length += $_->get_attribute('colspan') || 0 for @thead_th; + $thead_length += $_->get_attribute('colspan', 1) || 0 for @thead_th; my @tfoot_td = $driver->find_elements('//table[@id="issues-table"]/tfoot/tr/td'); my $tfoot_length = 0; - $tfoot_length += $_->get_attribute('colspan') || 0 for @tfoot_td; + $tfoot_length += $_->get_attribute('colspan', 1) || 0 for @tfoot_td; my @tbody_td = $driver->find_elements('//table[@id="issues-table"]/tbody/tr[2]/td'); my $tbody_length = 0; @@ -231,6 +233,8 @@ subtest 'XSS vulnerabilities in pagination' => sub { like( $second_page->get_attribute('href'), qr{category=2%22%3E%3Cscript%3Ealert%28%27booh%21%27%29%3C%2Fscript%3E}, 'The second page should display the variables and attributes correctly URI escaped' ); push @cleanup, $patron, $patron->category, $patron->library; + + $driver->quit(); }; END { diff --git a/t/db_dependent/selenium/update_child_to_adult.t b/t/db_dependent/selenium/update_child_to_adult.t index 1443e91de8..1b20b8ba4d 100755 --- a/t/db_dependent/selenium/update_child_to_adult.t +++ b/t/db_dependent/selenium/update_child_to_adult.t @@ -134,7 +134,7 @@ subtest 'Update child to patron' => sub { $driver->find_element('//div[@id="toolbar"]/div[@class="btn-group"][last()]')->click; # More button group my $update_link = $driver->find_element('//a[@id="updatechild"]'); - is($update_link->get_attribute('data-toggle'), 'tooltip', q|The update link should have a data-toggle attribute => it's a tooltip, not clickable|); + is($update_link->get_attribute('data-toggle', 1), 'tooltip', q|The update link should have a data-toggle attribute => it's a tooltip, not clickable|); $update_link->click; like( $driver->get_current_url, qr{/members/moremember\.pl\?borrowernumber=$adult_borrowernumber$}, 'After clicking the link, nothing happens, no # in the URL'); }; @@ -142,6 +142,9 @@ subtest 'Update child to patron' => sub { my @patrons = ( $adult_1, $adult_2, $child ); push @cleanup, $_, $_->library, for @patrons; push @cleanup, $patron_category_A, $patron_category_C; + + + $driver->quit(); }; END { diff --git a/t/lib/Selenium.pm b/t/lib/Selenium.pm index 261502aae1..ee25a649f7 100644 --- a/t/lib/Selenium.pm +++ b/t/lib/Selenium.pm @@ -58,6 +58,7 @@ sub new { ); bless $self, $class; $self->add_error_handler; + $self->driver->set_implicit_wait_timeout(5000); return $self; } @@ -73,6 +74,7 @@ sub add_error_handler { } print STDERR "\n"; $self->capture( $driver ); + $driver->quit(); croak $selenium_error; } ); @@ -103,8 +105,8 @@ sub auth { $self->driver->get($mainpage); $self->fill_form( { userid => $login, password => $password } ); - my $login_button = $self->driver->find_element('//input[@id="submit"]'); - $login_button->submit(); + my $login_button = $self->driver->find_element('//input[@id="submit-button"]'); + $login_button->click(); } sub opac_auth { @@ -137,7 +139,7 @@ sub submit_form { my ( $self ) = @_; my $default_submit_selector = '//fieldset[@class="action"]/input[@type="submit"]'; - $self->click_when_visible( $default_submit_selector ); + $self->driver->find_element($default_submit_selector)->click } sub click { @@ -169,13 +171,12 @@ sub click { if ( exists $params->{id} ) { $xpath_selector .= '//*[@id="'.$params->{id}.'"]'; } - $self->click_when_visible( $xpath_selector ); + $self->driver->find_element($xpath_selector)->click } sub wait_for_element_visible { my ( $self, $xpath_selector ) = @_; - $self->driver->set_implicit_wait_timeout(20000); my ($visible, $elt); $self->remove_error_handler; while ( not $visible ) { -- 2.29.2