Skip to content

Commit 3731267

Browse files
authored
Remove scroller.js and let Selenium do its thing (#2161)
Remove custom JS to handle scrolling and let selenium do its thing. Selenium handles scrolling elements nativly before clicking / sending text etc as part of the WebDriver spec. https://www.w3.org/TR/webdriver2/#element-click > The Element Click command scrolls into view the element if it is not already pointer-interactable, and clicks its in-view center point.
1 parent dc235bc commit 3731267

3 files changed

Lines changed: 8 additions & 75 deletions

File tree

src/main/java/org/jenkinsci/test/acceptance/po/ConfigurablePageObject.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.concurrent.Callable;
3434
import org.jenkinsci.test.acceptance.selenium.Scroller;
3535
import org.openqa.selenium.By;
36+
import org.openqa.selenium.Keys;
3637
import org.openqa.selenium.WebElement;
3738

3839
/**
@@ -132,6 +133,12 @@ public void ensureConfigPage() {
132133

133134
public void save() {
134135
WebElement e = find(SAVE_BUTTON);
136+
// form validation may happen if a currently focused element loosed focus
137+
// this can cause the position of the save button to change between the mouse down and mouse up
138+
// causing the click to be lost.
139+
// see https://github.com/mozilla/geckodriver/issues/2234 for more information
140+
// give the save button focus by sending it a "shift key" which is mostly harmless
141+
e.sendKeys(Keys.SHIFT);
135142
e.click();
136143
waitFor(e).until(CapybaraPortingLayerImpl::isStale);
137144
}

src/main/java/org/jenkinsci/test/acceptance/selenium/Scroller.java

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,13 @@
44
import java.io.InputStream;
55
import java.net.URL;
66
import java.nio.charset.StandardCharsets;
7-
import java.time.Duration;
8-
import java.util.Objects;
97
import java.util.logging.Level;
108
import java.util.logging.Logger;
11-
import org.jenkinsci.test.acceptance.junit.Wait;
12-
import org.openqa.selenium.By;
139
import org.openqa.selenium.JavascriptExecutor;
1410
import org.openqa.selenium.NoAlertPresentException;
15-
import org.openqa.selenium.TimeoutException;
1611
import org.openqa.selenium.UnhandledAlertException;
1712
import org.openqa.selenium.WebDriver;
1813
import org.openqa.selenium.WebDriver.Navigation;
19-
import org.openqa.selenium.WebElement;
2014
import org.openqa.selenium.support.events.WebDriverListener;
2115

2216
/**
@@ -74,39 +68,20 @@
7468
* @author Kohsuke Kawaguchi
7569
*/
7670
public class Scroller implements WebDriverListener {
77-
7871
private final Logger LOGGER = Logger.getLogger(Scroller.class.getName());
7972

80-
private final String scrollJs;
8173
private final String disableStickyElementsJs;
8274
private final WebDriver driver;
8375

8476
public Scroller(WebDriver driver) {
8577
this.driver = driver;
86-
try (InputStream scroller = getClass().getResourceAsStream("scroller.js");
87-
InputStream sticky = getClass().getResourceAsStream("disable-sticky-elements.js")) {
88-
scrollJs = new String(scroller.readAllBytes(), StandardCharsets.UTF_8);
78+
try (InputStream sticky = getClass().getResourceAsStream("disable-sticky-elements.js")) {
8979
disableStickyElementsJs = new String(sticky.readAllBytes(), StandardCharsets.UTF_8);
9080
} catch (IOException e) {
9181
throw new Error("Failed to load the JavaScript file", e);
9282
}
9383
}
9484

95-
@Override
96-
public void beforeClick(WebElement element) {
97-
scrollIntoView(element);
98-
}
99-
100-
@Override
101-
public void beforeSendKeys(WebElement element, CharSequence[] keysToSend) {
102-
scrollIntoView(element);
103-
}
104-
105-
@Override
106-
public void beforeClear(WebElement element) {
107-
scrollIntoView(element);
108-
}
109-
11085
@Override
11186
public void afterGet(WebDriver driver, String url) {
11287
disableStickyElementsIgnoringDialogs();
@@ -150,34 +125,4 @@ public void disableStickyElements() {
150125
unexpected);
151126
}
152127
}
153-
154-
/**
155-
* The framework is expected to take care of the correct scrolling. When you are tempted to scroll from PageObjects
156-
* or tests, there is likely a framework problem to be fixed.
157-
*/
158-
public void scrollIntoView(WebElement e) {
159-
WebElement element = e;
160-
if (Objects.equals(element.getTagName(), "option")) {
161-
element = e.findElement(By.xpath("..")); // scroll select into view not option
162-
}
163-
164-
final int eYCoord = element.getLocation().getY();
165-
final int eXCoord = element.getLocation().getX();
166-
final String id = element.getAttribute("id");
167-
final JavascriptExecutor executor = (JavascriptExecutor) driver;
168-
// Wait until web element is successfully scrolled.
169-
try {
170-
new Wait<>(Boolean.TRUE)
171-
.withTimeout(Duration.ofSeconds(5)) // Wall-clock time
172-
.until(() -> (Boolean) executor.executeScript(scrollJs, eYCoord, eXCoord, id));
173-
} catch (TimeoutException ex) {
174-
// Scrolling failed, but sometimes the element to click is already visible, let the test continue and
175-
// eventually fail later
176-
// This log message should be sufficient to diagnose the issue
177-
LOGGER.log(
178-
Level.WARNING,
179-
"Scrolling failed, letting the test to continue anyways, but \"Element is not clickable\" error will likely be thrown",
180-
ex);
181-
}
182-
}
183128
}

src/main/resources/org/jenkinsci/test/acceptance/selenium/scroller.js

Lines changed: 0 additions & 19 deletions
This file was deleted.

0 commit comments

Comments
 (0)