Skip to content

Commit a7204d0

Browse files
beldenfoxbeldenfox
committed
Merge remote-tracking branch 'upstream/master' into colormgmt
2 parents 0924592 + 4a33d5e commit a7204d0

File tree

12 files changed

+170
-57
lines changed

12 files changed

+170
-57
lines changed

‎modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/VirtualScrollBar.java

+3-29
Original file line numberDiff line numberDiff line change
@@ -128,36 +128,10 @@ public final BooleanProperty virtualProperty() {
128128
@Override public void adjustValue(double pos) {
129129
if (isVirtual()) {
130130
adjusting = true;
131-
double oldValue = flow.getPosition();
132-
double newValue = ((getMax() - getMin()) * Utils.clamp(0, pos, 1)) + getMin();
133-
/*
134-
* Scroll one cell further in the direction the user has clicked if only one cell is shown.
135-
* Otherwise, a click on the trough would have no effect when cell height > viewport height.
136-
*/
137-
IndexedCell<?> firstVisibleCell = flow.getFirstVisibleCell();
138-
IndexedCell<?> lastVisibleCell = flow.getLastVisibleCell();
139-
if (firstVisibleCell != null && firstVisibleCell == lastVisibleCell) {
140-
int index = firstVisibleCell.getIndex();
141-
if (newValue < oldValue) {
142-
index = Math.max(0, index - 1);
143-
} else {
144-
index = Math.min(flow.getCellCount(), index + 1);
145-
}
146-
flow.scrollTo(index);
131+
if (pos < getValue()) {
132+
flow.scrollPixels(-flow.getViewportLength());
147133
} else {
148-
if (newValue < oldValue) {
149-
IndexedCell<?> cell = firstVisibleCell;
150-
if (cell == null) {
151-
return;
152-
}
153-
flow.scrollToBottom(cell);
154-
} else if (newValue > oldValue) {
155-
IndexedCell<?> cell = lastVisibleCell;
156-
if (cell == null) {
157-
return;
158-
}
159-
flow.scrollToTop(cell);
160-
}
134+
flow.scrollPixels(flow.getViewportLength());
161135
}
162136
adjusting = false;
163137
} else {

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java

+9-14
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,15 @@ public class MenuBarSkin extends SkinBase<MenuBar> {
121121
* *
122122
**************************************************************************/
123123

124+
/** contains MenuBarButton's children only */
124125
private final HBox container;
126+
/** index of *focused* MenuBarButton */
127+
private int focusedMenuIndex = -1;
125128

126129
// represents the currently _open_ menu
127130
private Menu openMenu;
128131
private MenuBarButton openMenuButton;
129132

130-
// represents the currently _focused_ menu. If openMenu is non-null, this should equal
131-
// openMenu. If openMenu is null, this can be any menu in the menu bar.
132-
private Menu focusedMenu;
133-
private int focusedMenuIndex = -1;
134-
135133
private static WeakHashMap<Stage, Reference<MenuBarSkin>> systemMenuMap;
136134
private static List<MenuBase> wrappedDefaultMenus = new ArrayList<>();
137135
private static Stage currentMenuBarStage;
@@ -295,7 +293,7 @@ public MenuBarSkin(final MenuBar control) {
295293
// Key navigation
296294
sceneListenerHelper.addEventFilter(scene, KeyEvent.KEY_PRESSED, (ev) -> {
297295
// process right left and may be tab key events
298-
if (focusedMenu != null) {
296+
if (focusedMenuIndex >= 0) {
299297
switch (ev.getCode()) {
300298
case LEFT: {
301299
boolean isRTL = control.getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT;
@@ -429,7 +427,7 @@ private void showMenu(Menu menu, boolean selectFirstItem) {
429427
if (!menu.isShowing() && !isMenuEmpty(menu)) {
430428
if (selectFirstItem) {
431429
// put selection / focus on first item in menu
432-
MenuButton menuButton = getNodeForMenu(focusedMenuIndex);
430+
MenuButton menuButton = menuBarButtonAt(focusedMenuIndex);
433431
Skin<?> skin = menuButton.getSkin();
434432
if (skin instanceof MenuButtonSkinBase) {
435433
((MenuButtonSkinBase)skin).requestFocusOnFirstMenuItem();
@@ -445,7 +443,6 @@ private void showMenu(Menu menu, boolean selectFirstItem) {
445443
*/
446444
void setFocusedMenuIndex(int index) {
447445
focusedMenuIndex = (index >= -1 && index < getSkinnable().getMenus().size()) ? index : -1;
448-
focusedMenu = (focusedMenuIndex != -1) ? getSkinnable().getMenus().get(index) : null;
449446

450447
if (focusedMenuIndex != -1) {
451448
openMenuButton = (MenuBarButton)container.getChildren().get(focusedMenuIndex);
@@ -733,12 +730,9 @@ public void dispose() {
733730
* *
734731
**************************************************************************/
735732

736-
// For testing purpose only.
737-
MenuButton getNodeForMenu(int i) {
738-
if (i < container.getChildren().size()) {
739-
return (MenuBarButton)container.getChildren().get(i);
740-
}
741-
return null;
733+
// package protected for testing purposes
734+
MenuBarButton menuBarButtonAt(int i) {
735+
return (MenuBarButton)container.getChildren().get(i);
742736
}
743737

744738
int getFocusedMenuIndex() {
@@ -1083,6 +1077,7 @@ private void menuModeEnd() {
10831077
}
10841078

10851079
private void moveToMenu(Direction dir, boolean doShow) {
1080+
Menu focusedMenu = menuBarButtonAt(focusedMenuIndex).menu;
10861081
boolean showNextMenu = doShow && focusedMenu.isShowing();
10871082
findSibling(dir, focusedMenuIndex).ifPresent(p -> {
10881083
setFocusedMenuIndex(p.getValue());

‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -1901,7 +1901,6 @@ private final double getViewportBreadth() {
19011901
* The length of the viewport portion of the VirtualFlow as computed
19021902
* during the layout pass. In a vertical flow this would be the same as the
19031903
* clip view height. In a horizontal flow this is the clip view width.
1904-
* The access on this variable is package ONLY FOR TESTING.
19051904
*/
19061905
private double viewportLength;
19071906
void setViewportLength(double value) {
@@ -1912,7 +1911,15 @@ void setViewportLength(double value) {
19121911
this.absoluteOffset = getPosition() * (estimatedSize - viewportLength);
19131912
recalculateEstimatedSize();
19141913
}
1915-
double getViewportLength() {
1914+
1915+
/**
1916+
* Returns the length of the viewport portion of the {@code VirtualFlow} as computed during the layout pass.
1917+
* For a vertical flow this is based on the height and for a horizontal flow on the width of the clip view.
1918+
*
1919+
* @return the viewport length in pixels
1920+
* @since 23
1921+
*/
1922+
public double getViewportLength() {
19161923
return viewportLength;
19171924
}
19181925

‎modules/javafx.controls/src/shims/java/javafx/scene/control/skin/MenuBarSkinShim.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class MenuBarSkinShim {
3737

3838
// can only access the getNodeForMenu method in MenuBarSkin from this package.
3939
public static MenuButton getNodeForMenu(MenuBarSkin skin, int i) {
40-
return skin.getNodeForMenu(i);
40+
return skin.menuBarButtonAt(i);
4141
}
4242

4343
public static Skin getPopupSkin(MenuButton mb) {

‎modules/javafx.controls/src/shims/java/javafx/scene/control/skin/VirtualFlowShim.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,6 @@ public class VirtualFlowShim<T extends IndexedCell> extends VirtualFlow<T> {
3838
public final ArrayLinkedList<T> cells = super.cells;
3939
public final ObservableList<Node> sheetChildren = super.sheetChildren;
4040

41-
@Override
42-
public double getViewportLength() {
43-
return super.getViewportLength();
44-
}
45-
4641
@Override
4742
public void setViewportLength(double value) {
4843
super.setViewportLength(value);
@@ -58,6 +53,11 @@ public double getCellPosition(T cell) {
5853
return super.getCellPosition(cell);
5954
}
6055

56+
@Override
57+
public double getCellSize(int idx) {
58+
return super.getCellSize(idx);
59+
}
60+
6161
@Override
6262
public void setCellDirty(int idx) {
6363
super.setCellDirty(idx);

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/MenuBarTest.java

+65-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -716,4 +716,68 @@ protected void startApp(Parent root) {
716716
int focusedIndex = MenuBarSkinShim.getFocusedMenuIndex(skin);
717717
assertEquals(0, focusedIndex);
718718
}
719+
720+
/** Tests keyboard navigation with invisible menu JDK-8330304 */
721+
@Test
722+
public void testKeyNavigationWithInvisibleMenuItem() {
723+
Menu menu1 = mkMenu("1", 3);
724+
Menu menu2 = mkMenu("2", 3);
725+
Menu menu3 = mkMenu("3", 3);
726+
Menu menu4 = mkMenu("4", 3);
727+
menuBar.getMenus().setAll(menu1, menu2, menu3, menu4);
728+
729+
menu2.setVisible(false);
730+
731+
VBox root = new VBox(menuBar);
732+
startApp(root);
733+
tk.firePulse();
734+
735+
MenuBarSkin skin = (MenuBarSkin)menuBar.getSkin();
736+
assertTrue(skin != null);
737+
738+
double x = (menuBar.localToScene(menuBar.getLayoutBounds())).getMinX();
739+
double y = (menuBar.localToScene(menuBar.getLayoutBounds())).getMinY();
740+
741+
// show menu1 using mouse
742+
MenuButton mb = MenuBarSkinShim.getNodeForMenu(skin, 0);
743+
mb.getScene().getWindow().requestFocus();
744+
SceneHelper.processMouseEvent(scene,
745+
MouseEventGenerator.generateMouseEvent(MouseEvent.MOUSE_PRESSED, x+20, y+20));
746+
SceneHelper.processMouseEvent(scene,
747+
MouseEventGenerator.generateMouseEvent(MouseEvent.MOUSE_RELEASED, x+20, y+20));
748+
assertTrue(menu1.isShowing());
749+
750+
// show menu3 with RIGHT key press, menu2 is hidden and should be skipped
751+
KeyEventFirer keyboard = new KeyEventFirer(mb.getScene());
752+
keyboard.doKeyPress(KeyCode.RIGHT);
753+
tk.firePulse();
754+
755+
assertTrue(menu3.isShowing());
756+
757+
// show menu4
758+
keyboard.doKeyPress(KeyCode.RIGHT);
759+
tk.firePulse();
760+
761+
assertTrue(menu4.isShowing());
762+
763+
// show menu3
764+
keyboard.doKeyPress(KeyCode.LEFT);
765+
tk.firePulse();
766+
767+
assertTrue(menu3.isShowing());
768+
769+
// show menu1 (menu2 is hidden)
770+
keyboard.doKeyPress(KeyCode.LEFT);
771+
tk.firePulse();
772+
773+
assertTrue(menu1.isShowing());
774+
}
775+
776+
private Menu mkMenu(String name, int itemCount) {
777+
Menu m = new Menu("Menu" + name);
778+
for (int i = 0; i < itemCount; i++) {
779+
m.getItems().add(new MenuItem("Menu" + name + "Item" + i));
780+
}
781+
return m;
782+
}
719783
}

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java

+73
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@
5555

5656

5757
import java.util.List;
58+
import java.util.function.DoubleSupplier;
59+
import java.util.function.Supplier;
60+
5861
import javafx.scene.control.IndexedCellShim;
5962
import javafx.scene.control.ScrollBar;
6063
import javafx.scene.control.skin.VirtualFlowShim;
@@ -1838,6 +1841,76 @@ public void testScrollBarValueAdjustmentShouldScrollOneUp() {
18381841
assertEquals(3, flow.getFirstVisibleCell().getIndex());
18391842
}
18401843

1844+
@Test
1845+
public void testScrollBarValueAdjustmentMovementUp() {
1846+
testScrollBarValueAdjustment(1, 1.0, 0.2, () -> -flow.getViewportLength());
1847+
testScrollBarValueAdjustment(3, 1.0, 0.2, () -> -flow.getViewportLength());
1848+
testScrollBarValueAdjustment(1, 0.5, 0.2, () -> -flow.getViewportLength());
1849+
testScrollBarValueAdjustment(3, 0.5, 0.2, () -> -flow.getViewportLength());
1850+
}
1851+
1852+
@Test
1853+
public void testScrollBarValueAdjustmentMovementDown() {
1854+
testScrollBarValueAdjustment(1, 0.0, 0.8, () -> flow.getViewportLength());
1855+
testScrollBarValueAdjustment(3, 0.0, 0.8, () -> flow.getViewportLength());
1856+
testScrollBarValueAdjustment(1, 0.5, 0.8, () -> flow.getViewportLength());
1857+
testScrollBarValueAdjustment(3, 0.5, 0.8, () -> flow.getViewportLength());
1858+
}
1859+
1860+
public void testScrollBarValueAdjustment(int cellCount, double position, double adjust, DoubleSupplier targetMovement) {
1861+
flow = new VirtualFlowShim<>();
1862+
class C extends CellStub {
1863+
public C(VirtualFlowShim flow) {
1864+
super(flow);
1865+
}
1866+
1867+
@Override
1868+
protected double computePrefHeight(double width) {
1869+
return getIndex() == 0 ? 1000 : 100;
1870+
}
1871+
1872+
@Override
1873+
protected double computeMinHeight(double width) {
1874+
return computePrefHeight(width);
1875+
}
1876+
1877+
@Override
1878+
protected double computeMaxHeight(double width) {
1879+
return computePrefHeight(width);
1880+
}
1881+
}
1882+
flow.setCellFactory(fw -> new C(flow));
1883+
flow.setCellCount(cellCount);
1884+
flow.resize(256, 200);
1885+
1886+
flow.setPosition(position);
1887+
pulse();
1888+
1889+
Supplier<double[]> cellPositionsCalculater = () -> {
1890+
double[] positions = new double[cellCount];
1891+
IndexedCell<?> cell = flow.getFirstVisibleCell();
1892+
positions[cell.getIndex()] = flow.getCellPosition(cell);
1893+
for (int i = cell.getIndex() + 1; i < cellCount; i++) {
1894+
positions[i] = positions[i - 1] + flow.getCellSize(i - 1);
1895+
}
1896+
for (int i = cell.getIndex() - 1; i >= 0; i--) {
1897+
positions[i] = positions[i + 1] - flow.getCellSize(i);
1898+
}
1899+
return positions;
1900+
};
1901+
1902+
double[] positionsBefore = cellPositionsCalculater.get();
1903+
1904+
flow.shim_getVbar().adjustValue(adjust);
1905+
pulse();
1906+
1907+
double[] positionsAfter = cellPositionsCalculater.get();
1908+
1909+
for (int i = 0; i < positionsBefore.length; i++) {
1910+
assertEquals(targetMovement.getAsDouble(), positionsBefore[i] - positionsAfter[i], 0.1);
1911+
}
1912+
}
1913+
18411914
/**
18421915
* The first cell should always be the same when scrolling down just a little bit.
18431916
* This is mainly a regression test to check that no leading cells are added where they should not be.

‎modules/javafx.graphics/src/main/java/javafx/animation/FadeTransition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
/**
3737
* This {@code Transition} creates a fade effect animation that spans its
3838
* {@code duration}. This is done by updating the {@code opacity} variable of
39-
* the {@code node} at regular interval.
39+
* the {@code node} at regular intervals.
4040
* <p>
4141
* It starts from the {@code fromValue} if provided else uses the {@code node}'s
4242
* {@code opacity} value.

‎modules/javafx.graphics/src/main/java/javafx/animation/PathTransition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
* {@link #durationProperty() duration}. The translation along the path is done by updating the
4343
* {@code translateX} and {@code translateY} variables of the {@code node}, and
4444
* the {@code rotate} variable will get updated if {@code orientation} is set to
45-
* {@code OrientationType.ORTHOGONAL_TO_TANGENT}, at regular interval.
45+
* {@code OrientationType.ORTHOGONAL_TO_TANGENT}, at regular intervals.
4646
* <p>
4747
* The animated path is defined by the outline of a shape.
4848
*

‎modules/javafx.graphics/src/main/java/javafx/animation/RotateTransition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
/**
3838
* This {@code Transition} creates a rotation animation that spans its
3939
* {@code duration}. This is done by updating the {@code rotate} variable of the
40-
* {@code node} at regular interval. The angle value is specified in degrees.
40+
* {@code node} at regular intervals. The angle value is specified in degrees.
4141
* <p>
4242
* It starts from the {@code fromAngle} if provided else uses the {@code node}'s
4343
* {@code rotate} value.

‎modules/javafx.graphics/src/main/java/javafx/animation/ScaleTransition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* This {@code Transition} creates a scale animation that spans its
3838
* {@link #durationProperty() duration}. This is done by updating the {@code scaleX},
3939
* {@code scaleY} and {@code scaleZ} variables of the {@code node} at regular
40-
* interval.
40+
* intervals.
4141
* <p>
4242
* It starts from the ({@code fromX}, {@code fromY}, {@code fromZ}) value if
4343
* provided else uses the {@code node}'s ({@code scaleX}, {@code scaleY},

‎modules/javafx.graphics/src/main/java/javafx/animation/TranslateTransition.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* This {@code Transition} creates a move/translate animation that spans its
3838
* {@link #durationProperty() duration}. This is done by updating the {@code translateX},
3939
* {@code translateY} and {@code translateZ} variables of the {@code node} at
40-
* regular interval.
40+
* regular intervals.
4141
* <p>
4242
* It starts from the ({@code fromX}, {@code fromY}, {@code fromZ}) value if
4343
* provided else uses the {@code node}'s ({@code translateX}, {@code translateY}, {@code translateZ}) value.

0 commit comments

Comments
 (0)