Skip to content

Commit

Permalink
Fix screen-out-of-bounds (JabRef#11441)
Browse files Browse the repository at this point in the history 
* Fix window decorations

* Enable debug log

* Check all screens

* Debug output screens

* Always save new window position

* Save window state after fixing it

* Begin to support storing x and y positions for multiple screens

* Fix casing

* Rename constants

* Add initial file

* Discard changes to build.gradle

* Remove record WindowPreferences

* Refine debug log

* Fix preference variable name

* Fix typos

* Fix inbounds detection

* More logging

Co-authored-by: Christoph <[email protected]>

* Fix checkstyle

---------

Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: Christoph <[email protected]>
  • Loading branch information
@koppor @calixtus @Siedlerchr
3 people authored Aug 2, 2024
1 parent b8acb3c commit 072d7c7
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 47 deletions.
82 changes: 55 additions & 27 deletions src/main/java/org/jabref/gui/JabRefGUI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,29 +181,33 @@ private void openWindow() {
LOGGER.debug("Initializing frame");

GuiPreferences guiPreferences = preferencesService.getGuiPreferences();
LOGGER.debug("Reading from prefs: isMaximized {}", guiPreferences.isWindowMaximised());

mainStage.setMinHeight(330);
mainStage.setMinWidth(580);
mainStage.setMinHeight(330);

// maximized target state is stored, because "saveWindowState" saves x and y only if not maximized
boolean windowMaximised = guiPreferences.isWindowMaximised();

LOGGER.debug("Screens: {}", Screen.getScreens());
debugLogWindowState(mainStage);

if (isWindowPositionOutOfBounds()) {
LOGGER.debug("The JabRef window is outside of screen bounds. Position and size will be corrected. Main screen will be used.");
Rectangle2D bounds = Screen.getPrimary().getBounds();
mainStage.setX(bounds.getMinX());
mainStage.setY(bounds.getMinY());
mainStage.setHeight(Math.min(bounds.getHeight(), 786.0));
mainStage.setWidth(Math.min(bounds.getWidth(), 1024.0));
saveWindowState();
} else {
if (isWindowPositionInBounds()) {
LOGGER.debug("The JabRef window is inside screen bounds.");
mainStage.setX(guiPreferences.getPositionX());
mainStage.setY(guiPreferences.getPositionY());
mainStage.setWidth(guiPreferences.getSizeX());
mainStage.setHeight(guiPreferences.getSizeY());
LOGGER.debug("NOT saving window positions");
} else {
LOGGER.info("The JabRef window is outside of screen bounds. Position and size will be corrected to 1024x768. Primary screen will be used.");
Rectangle2D bounds = Screen.getPrimary().getBounds();
mainStage.setX(bounds.getMinX());
mainStage.setY(bounds.getMinY());
mainStage.setWidth(Math.min(bounds.getWidth(), 1024.0));
mainStage.setHeight(Math.min(bounds.getHeight(), 786.0));
LOGGER.debug("Saving window positions");
saveWindowState();
}
// after calling "saveWindowState" the maximized state can be set
mainStage.setMaximized(windowMaximised);
Expand Down Expand Up @@ -264,32 +268,56 @@ private void saveWindowState() {
}

/**
* outprints the Data from the Screen (only in debug mode)
* prints the data from the screen (only in debug mode)
*
* @param mainStage JabRefs stage
* @param mainStage JabRef's stage
*/
private void debugLogWindowState(Stage mainStage) {
if (LOGGER.isDebugEnabled()) {
String debugLogString = "SCREEN DATA:" +
"mainStage.WINDOW_MAXIMISED: " + mainStage.isMaximized() + "\n" +
"mainStage.POS_X: " + mainStage.getX() + "\n" +
"mainStage.POS_Y: " + mainStage.getY() + "\n" +
"mainStage.SIZE_X: " + mainStage.getWidth() + "\n" +
"mainStages.SIZE_Y: " + mainStage.getHeight() + "\n";
LOGGER.debug(debugLogString);
}
LOGGER.debug("""
screen data:
mainStage.WINDOW_MAXIMISED: {}
mainStage.POS_X: {}
mainStage.POS_Y: {}
mainStage.SIZE_X: {}
mainStage.SIZE_Y: {}
""",
mainStage.isMaximized(), mainStage.getX(), mainStage.getY(), mainStage.getWidth(), mainStage.getHeight());
}

/**
* Tests if the window coordinates are out of the mainscreen
* Tests if the window coordinates are inside any screen
*/
private boolean isWindowPositionOutOfBounds() {
// The upper right corner is checked as there are most probably the window controls.
private boolean isWindowPositionInBounds() {
GuiPreferences guiPreferences = preferencesService.getGuiPreferences();
double rightX = guiPreferences.getPositionX() + guiPreferences.getSizeX();

if (LOGGER.isDebugEnabled()) {
Screen.getScreens().forEach(screen -> LOGGER.debug("Screen bounds: {}", screen.getBounds()));
}

return lowerLeftIsInBounds(guiPreferences) && upperRightIsInBounds(guiPreferences);
}

private boolean lowerLeftIsInBounds(GuiPreferences guiPreferences) {
// Windows/PowerToys somehow removes 10 pixels to the left; they are re-added
double leftX = guiPreferences.getPositionX() + 10.0;
double bottomY = guiPreferences.getPositionY() + guiPreferences.getSizeY();
LOGGER.debug("left x: {}, bottom y: {}", leftX, bottomY);

boolean inBounds = Screen.getScreens().stream().anyMatch((screen -> screen.getBounds().contains(leftX, bottomY)));
LOGGER.debug("lower left corner is in bounds: {}", inBounds);
return inBounds;
}

private boolean upperRightIsInBounds(GuiPreferences guiPreferences) {
// The upper right corner is checked as there are most probably the window controls.
// Windows/PowerToys somehow adds 10 pixels to the right and top of the screen, they are removed
double rightX = guiPreferences.getPositionX() + guiPreferences.getSizeX() - 10.0;
double topY = guiPreferences.getPositionY();
return Screen.getScreens().stream().noneMatch((screen -> screen.getBounds().contains(
rightX, topY)));
LOGGER.debug("right x: {}, top y: {}", rightX, topY);

boolean inBounds = Screen.getScreens().stream().anyMatch((screen -> screen.getBounds().contains(rightX, topY)));
LOGGER.debug("upper right corner is in bounds: {}", inBounds);
return inBounds;
}

// Background tasks
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/preferences/GuiPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@ public class GuiPreferences {

private final BooleanProperty windowMaximised;

private final DoubleProperty sidePaneWidth;

// the last libraries that were open when jabref closes and should be reopened on startup
private final ObservableList<Path> lastFilesOpened;

private final ObjectProperty<Path> lastFocusedFile;

// observable list last files opened in the file menu
private final FileHistory fileHistory;

private final StringProperty lastSelectedIdBasedFetcher;
private final DoubleProperty sidePaneWidth;

public GuiPreferences(double positionX,
double positionY,
Expand Down
43 changes: 24 additions & 19 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ public class JabRefPreferences implements PreferencesService {
public static final String XMP_PRIVACY_FILTERS = "xmpPrivacyFilters";
public static final String USE_XMP_PRIVACY_FILTER = "useXmpPrivacyFilter";
public static final String DEFAULT_SHOW_SOURCE = "defaultShowSource";
// Window sizes
public static final String SIZE_Y = "mainWindowSizeY";
public static final String SIZE_X = "mainWindowSizeX";
public static final String POS_Y = "mainWindowPosY";
public static final String POS_X = "mainWindowPosX";

public static final String MAIN_WINDOW_POS_X = "mainWindowPosX";
public static final String MAIN_WINDOW_POS_Y = "mainWindowPosY";
public static final String MAIN_WINDOW_WIDTH = "mainWindowSizeX";
public static final String MAIN_WINDOW_HEIGHT = "mainWindowSizeY";

public static final String LAST_EDITED = "lastEdited";
public static final String OPEN_LAST_EDITED = "openLastEdited";
Expand Down Expand Up @@ -614,12 +614,16 @@ private JabRefPreferences() {
.getSslDirectory()
.resolve("truststore.jks").toString());

defaults.put(POS_X, 0);
defaults.put(POS_Y, 0);
defaults.put(SIZE_X, 1024);
defaults.put(SIZE_Y, 768);
defaults.put(MAIN_WINDOW_POS_X, 0);
defaults.put(MAIN_WINDOW_POS_Y, 0);
defaults.put(MAIN_WINDOW_WIDTH, 1024);
defaults.put(MAIN_WINDOW_HEIGHT, 768);

defaults.put(WINDOW_MAXIMISED, Boolean.TRUE);
defaults.put(AUTO_RESIZE_MODE, Boolean.FALSE); // By default disable "Fit table horizontally on the screen"

// By default disable "Fit table horizontally on the screen"
defaults.put(AUTO_RESIZE_MODE, Boolean.FALSE);

defaults.put(ENTRY_EDITOR_HEIGHT, 0.65);
defaults.put(ENTRY_EDITOR_PREVIEW_DIVIDER_POS, 0.5);
defaults.put(NAMES_AS_IS, Boolean.FALSE); // "Show names unchanged"
Expand Down Expand Up @@ -2610,10 +2614,10 @@ public GuiPreferences getGuiPreferences() {
}

guiPreferences = new GuiPreferences(
getDouble(POS_X),
getDouble(POS_Y),
getDouble(SIZE_X),
getDouble(SIZE_Y),
getDouble(MAIN_WINDOW_POS_X),
getDouble(MAIN_WINDOW_POS_Y),
getDouble(MAIN_WINDOW_WIDTH),
getDouble(MAIN_WINDOW_HEIGHT),
getBoolean(WINDOW_MAXIMISED),
getStringList(LAST_EDITED).stream()
.map(Path::of)
Expand All @@ -2623,11 +2627,13 @@ public GuiPreferences getGuiPreferences() {
get(ID_ENTRY_GENERATOR),
getDouble(SIDE_PANE_WIDTH));

EasyBind.listen(guiPreferences.positionXProperty(), (obs, oldValue, newValue) -> putDouble(POS_X, newValue.doubleValue()));
EasyBind.listen(guiPreferences.positionYProperty(), (obs, oldValue, newValue) -> putDouble(POS_Y, newValue.doubleValue()));
EasyBind.listen(guiPreferences.sizeXProperty(), (obs, oldValue, newValue) -> putDouble(SIZE_X, newValue.doubleValue()));
EasyBind.listen(guiPreferences.sizeYProperty(), (obs, oldValue, newValue) -> putDouble(SIZE_Y, newValue.doubleValue()));
EasyBind.listen(guiPreferences.positionXProperty(), (obs, oldValue, newValue) -> putDouble(MAIN_WINDOW_POS_X, newValue.doubleValue()));
EasyBind.listen(guiPreferences.positionYProperty(), (obs, oldValue, newValue) -> putDouble(MAIN_WINDOW_POS_Y, newValue.doubleValue()));
EasyBind.listen(guiPreferences.sizeXProperty(), (obs, oldValue, newValue) -> putDouble(MAIN_WINDOW_WIDTH, newValue.doubleValue()));
EasyBind.listen(guiPreferences.sizeYProperty(), (obs, oldValue, newValue) -> putDouble(MAIN_WINDOW_HEIGHT, newValue.doubleValue()));
EasyBind.listen(guiPreferences.windowMaximisedProperty(), (obs, oldValue, newValue) -> putBoolean(WINDOW_MAXIMISED, newValue));
EasyBind.listen(guiPreferences.sidePaneWidthProperty(), (obs, oldValue, newValue) -> putDouble(SIDE_PANE_WIDTH, newValue.doubleValue()));

guiPreferences.getLastFilesOpened().addListener((ListChangeListener<Path>) change -> {
if (change.getList().isEmpty()) {
prefs.remove(LAST_EDITED);
Expand All @@ -2647,7 +2653,6 @@ public GuiPreferences getGuiPreferences() {
});
guiPreferences.getFileHistory().addListener((InvalidationListener) change -> storeFileHistory(guiPreferences.getFileHistory()));
EasyBind.listen(guiPreferences.lastSelectedIdBasedFetcherProperty(), (obs, oldValue, newValue) -> put(ID_ENTRY_GENERATOR, newValue));
EasyBind.listen(guiPreferences.sidePaneWidthProperty(), (obs, oldValue, newValue) -> putDouble(SIDE_PANE_WIDTH, newValue.doubleValue()));

return guiPreferences;
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/tinylog.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ exception = strip: jdk.internal
#[email protected] = debug

[email protected] = debug

[email protected] = debug

0 comments on commit 072d7c7

Please sign in to comment.