From 42aab4343e9016864f640ff29e874e8f6702f665 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 (cherry picked from commit 3c3406257e78a9407320d22820807f767b4c4df3) Signed-off-by: Fridolin Somers (cherry picked from commit 51fcc453f9db5c196cc62cb29f3321f32f5dd54e) Signed-off-by: Andrew Fuerste-Henry (cherry picked from commit 09518a3773823e339304a3f187f1184b89ddf902) Signed-off-by: Victor Grousset/tuxayo --- 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 | 16 +++++++++++++--- t/db_dependent/selenium/patrons_search.t | 2 ++ t/db_dependent/selenium/regressions.t | 14 +++++++++----- t/db_dependent/selenium/update_child_to_adult.t | 5 ++++- t/lib/Selenium.pm | 11 ++++++----- 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/css/login.css b/koha-tmpl/intranet-tmpl/prog/css/login.css index 14f271421a..15e5340ea8 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/login.css +++ b/koha-tmpl/intranet-tmpl/prog/css/login.css @@ -117,7 +117,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 86db48ff66..5e3b1623fe 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -85,7 +85,7 @@ -

+

[% IF ( casAuthentication ) %]

Cas login

diff --git a/t/db_dependent/selenium/basic_workflow.t b/t/db_dependent/selenium/basic_workflow.t index 5f785d21df..0a83981c7d 100644 --- a/t/db_dependent/selenium/basic_workflow.t +++ b/t/db_dependent/selenium/basic_workflow.t @@ -35,6 +35,7 @@ use Modern::Perl; use Time::HiRes qw(gettimeofday); +use POSIX qw(strftime); use C4::Context; use C4::Biblio qw( AddBiblio ); # We shouldn't use it @@ -166,10 +167,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|; @@ -187,6 +187,16 @@ SKIP: { # It's a varchar(10) $v = 't_value_x'; } + elsif ( + $id =~ m|^tag_952_subfield_w| # replacementpricedate + ) { + $v = strftime("%Y-%m-%d", localtime); + } + elsif ( + $id =~ m|^tag_952_subfield_d| # dateaccessioned + ) { + next; # The input has been prefilled with %Y-%m-%d already + } else { $v = 't_value_bib' . $biblionumber; } diff --git a/t/db_dependent/selenium/patrons_search.t b/t/db_dependent/selenium/patrons_search.t index 1d8cd98e48..22a99746d4 100644 --- a/t/db_dependent/selenium/patrons_search.t +++ b/t/db_dependent/selenium/patrons_search.t @@ -114,6 +114,8 @@ subtest 'Search patrons' => sub { push @cleanup, $_ for @patrons; push @cleanup, $library; push @cleanup, $patron_category; + + $driver->quit(); }; END { diff --git a/t/db_dependent/selenium/regressions.t b/t/db_dependent/selenium/regressions.t index e33062fd65..b0f6555620 100644 --- 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; @@ -175,15 +177,15 @@ 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/td'); my $tbody_length = 0; - $tbody_length += $_->get_attribute('colspan') || 0 for @tbody_td; + $tbody_length += $_->get_attribute('colspan', 1) || 0 for @tbody_td; is( $thead_length == $tfoot_length && $tfoot_length == $tbody_length, 1, "Checkouts table must be correctly aligned" ) @@ -238,6 +240,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 100644 --- 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 1e70e94b34..7e8f61d5d1 100644 --- a/t/lib/Selenium.pm +++ b/t/lib/Selenium.pm @@ -52,6 +52,7 @@ sub new { ); bless $self, $class; $self->add_error_handler; + $self->driver->set_implicit_wait_timeout(5000); return $self; } @@ -67,6 +68,7 @@ sub add_error_handler { } print STDERR "\n"; $self->capture( $driver ); + $driver->quit(); croak $selenium_error; } ); @@ -97,8 +99,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 { @@ -131,7 +133,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 { @@ -163,13 +165,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.39.5