Merge pull request #104 from abayer/revert-jenkins-34411

Revert JENKINS-34411
jenkinsci · May 19, 2016 · e5ba520 · e5ba520
2 parents e11f164 + 1cdd098
commit e5ba520
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 101 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,9 +1,7 @@
 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;
@@ -63,73 +61,24 @@
  * @author Kohsuke Kawaguchi
  */
 public class Scroller extends AbstractWebDriverEventListener {
-    /** 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;
+    private final String scrollJs;
 
     public Scroller() throws IOException {
-        overflowJS = IOUtils.toString(Scroller.class.getResourceAsStream("overflow.js"));
+        scrollJs = IOUtils.toString(Scroller.class.getResourceAsStream("scroller.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) {
-        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);
-        }
+        int eYCoord = e.getLocation().getY();
+        ((JavascriptExecutor)driver).executeScript(scrollJs, eYCoord);
     }
 }
diff --git a/src/main/resources/org/jenkinsci/test/acceptance/selenium/overflow.js b/src/main/resources/org/jenkinsci/test/acceptance/selenium/overflow.js
diff --git a/src/main/resources/org/jenkinsci/test/acceptance/selenium/scroller.js b/src/main/resources/org/jenkinsci/test/acceptance/selenium/scroller.js
@@ -0,0 +1,8 @@
+// Visually navigate to the element in order to interact with it.
+
+var eYCoord = arguments[0];
+
+// Scroll to the element. It will appear at the top edge of the screen.
+// We subtract a bit so as to accommodate fixed position banners at the top
+// (e.g. breadcrumbs, tabbars etc), making sure they are not hiding the element.
+window.scrollTo(0, eYCoord - 200);
diff --git a/src/test/java/plugins/MatrixAuthPluginTest.java b/src/test/java/plugins/MatrixAuthPluginTest.java
@@ -10,7 +10,6 @@
 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;
@@ -110,36 +109,4 @@ 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();
-    }
 }