8276313: ScrollPane scroll delta incorrectly depends on content height

Reviewed-by: kcr, aghaisas, jvos

Author: Michael Strauß

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java

@@ -891,13 +891,8 @@ else if (newVVal < vsb.getMin()) {
             */
             if (vsb.getVisibleAmount() < vsb.getMax()) {
                 double vRange = getSkinnable().getVmax()-getSkinnable().getVmin();
-                double vPixelValue;
-                if (nodeHeight > 0.0) {
-                    vPixelValue = vRange / nodeHeight;
-                }
-                else {
-                    vPixelValue = 0.0;
-                }
+                double hDelta = nodeHeight - contentHeight;
+                double vPixelValue = hDelta > 0.0 ? vRange / hDelta : 0.0;
                 double newValue = vsb.getValue()+(-event.getDeltaY())*vPixelValue;
                 if (!Properties.IS_TOUCH_SUPPORTED) {
                     if ((event.getDeltaY() > 0.0 && vsb.getValue() > vsb.getMin()) ||
@@ -923,14 +918,8 @@ else if (newVVal < vsb.getMin()) {
 
             if (hsb.getVisibleAmount() < hsb.getMax()) {
                 double hRange = getSkinnable().getHmax()-getSkinnable().getHmin();
-                double hPixelValue;
-                if (nodeWidth > 0.0) {
-                    hPixelValue = hRange / nodeWidth;
-                }
-                else {
-                    hPixelValue = 0.0;
-                }
-
+                double wDelta = nodeWidth - contentWidth;
+                double hPixelValue = wDelta > 0.0 ? hRange / wDelta : 0.0;
                 double newValue = hsb.getValue()+(-event.getDeltaX())*hPixelValue;
                 if (!Properties.IS_TOUCH_SUPPORTED) {
                     if ((event.getDeltaX() > 0.0 && hsb.getValue() > hsb.getMin()) ||
@@ -998,14 +987,16 @@ void scrollBoundsIntoView(Bounds b) {
         // But to do this we have to set the scrollbars' values appropriately.
 
         if (dx != 0) {
-            double sdx = dx * (hsb.getMax() - hsb.getMin()) / (nodeWidth - contentWidth);
+            double wd = nodeWidth - contentWidth;
+            double sdx = wd > 0.0 ? dx * (hsb.getMax() - hsb.getMin()) / wd : 0;
             // Adjust back for some amount so that the Node border is not too close to view border
             sdx += -1 * Math.signum(sdx) * hsb.getUnitIncrement() / 5; // This accounts to 2% of view width
             hsb.setValue(hsb.getValue() + sdx);
             getSkinnable().requestLayout();
         }
         if (dy != 0) {
-            double sdy = dy * (vsb.getMax() - vsb.getMin()) / (nodeHeight - contentHeight);
+            double hd = nodeHeight - contentHeight;
+            double sdy = hd > 0.0 ? dy * (vsb.getMax() - vsb.getMin()) / hd : 0.0;
             // Adjust back for some amount so that the Node border is not too close to view border
             sdy += -1 * Math.signum(sdy) * vsb.getUnitIncrement() / 5; // This accounts to 2% of view height
             vsb.setValue(vsb.getValue() + sdy);
@@ -1179,15 +1170,17 @@ private void updateVerticalSB() {
     private double updatePosX() {
         final ScrollPane sp = getSkinnable();
         double x = isReverseNodeOrientation() ? (hsb.getMax() - (posX - hsb.getMin())) : posX;
-        double minX = Math.min((- x / (hsb.getMax() - hsb.getMin()) * (nodeWidth - contentWidth)), 0);
+        double hsbRange = hsb.getMax() - hsb.getMin();
+        double minX = hsbRange > 0 ? Math.min(-x / hsbRange * (nodeWidth - contentWidth), 0) : 0;
         viewContent.setLayoutX(snapPositionX(minX));
         if (!sp.hvalueProperty().isBound()) sp.setHvalue(Utils.clamp(sp.getHmin(), posX, sp.getHmax()));
         return posX;
     }
 
     private double updatePosY() {
         final ScrollPane sp = getSkinnable();
-        double minY = Math.min((- posY / (vsb.getMax() - vsb.getMin()) * (nodeHeight - contentHeight)), 0);
+        double vsbRange = vsb.getMax() - vsb.getMin();
+        double minY = vsbRange > 0 ? Math.min(-posY / vsbRange * (nodeHeight - contentHeight), 0) : 0;
         viewContent.setLayoutY(snapPositionY(minY));
         if (!sp.vvalueProperty().isBound()) sp.setVvalue(Utils.clamp(sp.getVmin(), posY, sp.getVmax()));
         return posY;

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/ScrollPaneSkinTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -745,7 +745,56 @@ public void growW() {
         assertTrue(scrollPaneInner.getHvalue() > 0.0);
     }
 
+    @Test
+    public void testScrollDeltaIsIndependentOfScrollPaneHeight() {
+        var content = new Rectangle();
+        content.setHeight(200);
+        content.setWidth(100);
 
+        var scrollPane = new ScrollPane();
+        scrollPane.setSkin(new ScrollPaneSkinMock(scrollPane));
+        scrollPane.setContent(content);
+        scrollPane.setPrefWidth(100);
+        scrollPane.setPrefHeight(100);
+
+        Stage stage = new Stage();
+        stage.setScene(new Scene(new Group(scrollPane), 500, 500));
+        stage.show();
+
+        Event.fireEvent(content, new ScrollEvent(
+            ScrollEvent.SCROLL,
+            50, 50,
+            50, 50,
+            false, false, false, false, true, false,
+            0.0, -10.0, 0.0, -10.0,
+            ScrollEvent.HorizontalTextScrollUnits.NONE, 0.0,
+            ScrollEvent.VerticalTextScrollUnits.NONE, 0.0,
+            0, null));
+
+        double firstY = content.getLocalToSceneTransform().transform(0, 0).getY();
+        scrollPane.setPrefHeight(150);
+        scrollPane.setVvalue(0);
+
+        stage.close();
+        stage = new Stage();
+        stage.setScene(new Scene(new Group(scrollPane), 500, 500));
+        stage.show();
+
+        Event.fireEvent(content, new ScrollEvent(
+            ScrollEvent.SCROLL,
+            50, 50,
+            50, 50,
+            false, false, false, false, true, false,
+            0.0, -10.0, 0.0, -10.0,
+            ScrollEvent.HorizontalTextScrollUnits.NONE, 0.0,
+            ScrollEvent.VerticalTextScrollUnits.NONE, 0.0,
+            0, null));
+
+        double secondY = content.getLocalToSceneTransform().transform(0, 0).getY();
+        stage.close();
+
+        assertEquals(firstY, secondY, 0.001);
+    }
 
     public static final class ScrollPaneSkinMock extends ScrollPaneSkinShim {
         boolean propertyChanged = false;