Attachment 114725 Details for Bug 27055 – Bug 27055: Fix compatibility with newer Firefox+Selenium version

[patch] Bug 27055: Fix compatibility with newer Firefox+Selenium version

Bug-27055-Fix-compatibility-with-newer-FirefoxSele.patch (text/plain), 9.99 KB, created by Tomás Cohen Arazi (tcohen) on 2020-12-29 15:18:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-12-29 15:18:45 UTC

Size: 9.99 KB

patch

obsolete

>From 6d36c612ff39c3e942d221011a508f2835a62293 Mon Sep 17 00:00:00 2001
>From: Victor Grousset/tuxayo <victor@tuxayo.net>
>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 <mtj@kohaaloha.com>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> 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 @@
> 
> 
> 
>-<p class="submit"><input id="submit" type="submit" value="Login" tabindex="4" /></p>
>+<p class="submit"><input id="submit-button" type="submit" value="Login" tabindex="4" /></p>
> </form>
> [% IF ( casAuthentication ) %]
> <h4>Cas login</h4>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 27055: 113810 | 113922 | 114005 | 114584 | 114725