Merge pull request #97 from andresrc/JENKINS-34411

[JENKINS-34411] Horizontal scrolling and scrolling overflows
jenkinsci · May 9, 2016 · 45d11e5 · 45d11e5
2 parents 066f75f + ddc1dac
commit 45d11e5
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 12 deletions.
diff --git a/src/main/java/org/jenkinsci/test/acceptance/selenium/Scroller.java b/src/main/java/org/jenkinsci/test/acceptance/selenium/Scroller.java
@@ -1,7 +1,9 @@
 package org.jenkinsci.test.acceptance.selenium;
 
 import org.apache.commons.io.IOUtils;
+import org.openqa.selenium.By;
 import org.openqa.selenium.JavascriptExecutor;
+import org.openqa.selenium.Point;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.events.AbstractWebDriverEventListener;
@@ -61,24 +63,73 @@
  * @author Kohsuke Kawaguchi
  */
 public class Scroller extends AbstractWebDriverEventListener {
-    private final String scrollJs;
+    /** Horizontal margin to use when scrolling to the element. */
+    private static final int MARGIN_X = Integer.getInteger("SCROLL_MARGIN_X", 200);
+    /** Vertical margin to use when scrolling to the element. */
+    private static final int MARGIN_Y = Integer.getInteger("SCROLL_MARGIN_Y", 200);
+
+    private final String overflowJS;
 
     public Scroller() throws IOException {
-        scrollJs = IOUtils.toString(Scroller.class.getResourceAsStream("scroller.js"));
+        overflowJS = IOUtils.toString(Scroller.class.getResourceAsStream("overflow.js"));
     }
 
     @Override
     public void beforeClickOn(WebElement element, WebDriver driver) {
         scrollIntoView(element, driver);
     }
 
+    @Override
+    public void afterClickOn(WebElement element, WebDriver driver) {
+        // A click can cause a page change.
+        overideOverflow(driver);
+    }
+
     @Override
     public void beforeChangeValueOf(WebElement element, WebDriver driver) {
         scrollIntoView(element, driver);
     }
 
+    @Override
+    public void afterNavigateTo(String url, WebDriver driver) {
+        overideOverflow(driver);
+    }
+
+    @Override
+    public void afterNavigateBack(WebDriver driver) {
+        overideOverflow(driver);
+    }
+
+    @Override
+    public void afterNavigateForward(WebDriver driver) {
+        overideOverflow(driver);
+    }
+
+    @Override
+    public void afterNavigateRefresh(WebDriver driver) {
+        overideOverflow(driver);
+    }
+
+    /**
+     * To scroll the element to the view, we scroll the element to the top-edge of the screen.
+     * A (configurable) margin is left to account for decorations.
+     * Alternatives based on `scrollToView` and {@link org.openqa.selenium.interactions.Actions#moveToElement}
+     * were tested but did not solve JENKINS-34411.
+     * @param e Element to scroll to view.
+     * @param driver Driver instance.
+     */
     private void scrollIntoView(WebElement e, WebDriver driver) {
-        int eYCoord = e.getLocation().getY();
-        ((JavascriptExecutor)driver).executeScript(scrollJs, eYCoord);
+        final Point p = e.getLocation();
+        final int x = p.getX();
+        final int y = p.getY();
+        final String script = String.format("window.scrollTo(%d, %d);", p.getX() - MARGIN_X, p.getY() - MARGIN_Y);
+        ((JavascriptExecutor)driver).executeScript(script);
+    }
+
+    /** Override overflow behavior (if not done previously). */
+    private void overideOverflow(WebDriver driver) {
+        if (driver.findElements(By.id("ath-overflow-override")).isEmpty()) {
+            ((JavascriptExecutor)driver).executeScript(overflowJS);
+        }
     }
 }
diff --git a/src/main/resources/org/jenkinsci/test/acceptance/selenium/overflow.js b/src/main/resources/org/jenkinsci/test/acceptance/selenium/overflow.js
@@ -0,0 +1,13 @@
+var css = 'html body * { overflow: visible !important }',
+    head = document.head || document.getElementsByTagName('head')[0],
+    style = document.createElement('style');
+
+style.type = 'text/css';
+style.id = 'ath-overflow-override'
+if (style.styleSheet){
+  style.styleSheet.cssText = css;
+} else {
+  style.appendChild(document.createTextNode(css));
+}
+
+head.appendChild(style);
diff --git a/src/main/resources/org/jenkinsci/test/acceptance/selenium/scroller.js b/src/main/resources/org/jenkinsci/test/acceptance/selenium/scroller.js
diff --git a/src/test/java/plugins/MatrixAuthPluginTest.java b/src/test/java/plugins/MatrixAuthPluginTest.java
@@ -10,6 +10,7 @@
 import org.jenkinsci.test.acceptance.po.FreeStyleJob;
 import org.jenkinsci.test.acceptance.po.GlobalSecurityConfig;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 
 import static org.jenkinsci.test.acceptance.plugins.matrix_auth.MatrixRow.*;
 import static org.junit.Assert.assertNotNull;
@@ -109,4 +110,36 @@ public void projectMatrixAuth() throws Exception {
         jenkins.login().doLogin("bob");
         assertNotNull(getElement(by.href("job/"+j.name+"/")));
     }
+
+
+    /**
+     * In the context JENKINS-34411, the permissions out of view would not be activated,
+     * so the job creation would fail.
+     */
+    @Test
+    @Issue("JENKINS-34411")
+    public void developer() throws Exception {
+        GlobalSecurityConfig sc = new GlobalSecurityConfig(jenkins);
+        sc.open();
+        {
+            MockSecurityRealm ms = sc.useRealm(MockSecurityRealm.class);
+            ms.configure("alice","bob");
+
+            MatrixAuthorizationStrategy mas = sc.useAuthorizationStrategy(MatrixAuthorizationStrategy.class);
+
+            MatrixRow a = mas.addUser("alice");
+            a.admin();
+
+            MatrixRow bob = mas.addUser("bob");
+            bob.developer();
+        }
+        sc.save();
+
+        jenkins.login().doLogin("bob");
+
+        // just create the job without configuring
+        FreeStyleJob j = jenkins.jobs.create();
+        jenkins.open();
+        j.open();
+    }
 }