Bug 137471 - CMIS dialog advances beyond lower right corner of the screen
Summary: CMIS dialog advances beyond lower right corner of the screen
Status: REOPENED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.1.0.0.alpha0+
Hardware: All Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: CMIS KDE, KF5
  Show dependency treegraph
 
Reported: 2020-10-14 10:18 UTC by Heiko Tietze
Modified: 2024-03-18 06:50 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
screencast (12.10 MB, video/webm)
2020-11-02 12:46 UTC, Michael Weghorn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Heiko Tietze 2020-10-14 10:18:21 UTC
Follow-up to bug 128174: the CMIS dialog comes up centered of the main window at the first call and goes down to the lower right corner if shown again. The position goes even beyond the screen area making any interaction impossible.

Version: 7.1.0.0.alpha0+
Build ID: 59f86333f3fce091177d1cfb9363aa81686aa497
CPU threads: 8; OS: Linux 5.8; UI render: default; VCL: kf5
Locale: de-DE (en_US.UTF-8); UI: en-US
Calc: threaded
Comment 1 Heiko Tietze 2020-10-14 11:06:38 UTC
Issue is kf5-only, gen and gtk3 place the CMIS dialog always at the app center, gen places the master dialog apart close to the right side of the application.
Comment 2 Michael Weghorn 2020-10-16 19:48:28 UTC
(In reply to Heiko Tietze from comment #0)
> Follow-up to bug 128174: the CMIS dialog comes up centered of the main
> window at the first call and goes down to the lower right corner if shown
> again. The position goes even beyond the screen area making any interaction
> impossible.

Can you be a bit more explicit how to reproduce this?

I don't know much about the CMIS/Remote files functionality, just quickly tried, but for me the dialogs are always visible.

This screencast shows how it behaves for me on Debian testing (file will probably be deleted at some point in time in the future):
https://nextcloud.documentfoundation.org/s/zgQrpEDAEEGoHAF

Also, when choosing "File" -> "Open Remote...", then cancelling and doing the same a few times, the dialog is well visible in my case.

Version: 7.1.0.0.alpha0+
Build ID: 247b247dadc8f0133a8eb94f1423a29315cf998a
CPU threads: 12; OS: Linux 5.8; UI render: default; VCL: kf5
Locale: en-GB (en_GB.UTF-8); UI: en-US
Calc: threaded
Comment 3 Michael Weghorn 2020-10-16 19:51:56 UTC
FWIW, I basically followed [1] and [2] to quickly set up some Alfresco server with CMIS functionality in some Debian VM, URL I enter in the dialog to connect from within in the local network (NAT) is:
http://192.168.122.192:8080/alfresco/cmisws/RepositoryService?wsdl

[1] https://www.howtoforge.com/tutorial/how-to-install-alfresco-cms-on-ubuntu-1804/
[2] https://vienergie.wordpress.com/2014/10/26/integrating-libreoffice-4-and-alfresco-using-cmis-2/
Comment 4 Heiko Tietze 2020-10-17 08:43:01 UTC
(In reply to Michael Weghorn from comment #2)
> Can you be a bit more explicit how to reproduce this?

Within the session, cancel the master dialog (and the cmis dialog) and start again. I expect the CMIS dialog on center of the application (and the master pw dialog centered to the CMIS dialog, done recently by Ayhan) but every time I open the dialog new it advances down to the lower right border. Haven't tried other dialogs.
Comment 5 Heiko Tietze 2020-10-17 09:20:37 UTC
Actually, you don't have to make any choice in the CMIS dialog. File > Open Remote > Cancel - I can run it three times, then the dialog shows up out of my reach.
Comment 6 Michael Weghorn 2020-10-17 12:05:52 UTC
(In reply to Heiko Tietze from comment #4)
> Within the session, cancel the master dialog (and the cmis dialog) and start
> again. I expect the CMIS dialog on center of the application (and the master
> pw dialog centered to the CMIS dialog, done recently by Ayhan) but every
> time I open the dialog new it advances down to the lower right border.
> Haven't tried other dialogs.

In my case, the dialog does not leave the visible/clickable area. The window manager (KWin 5.17.4 in Plasma X11 session on Debian testing in my case) might play a role.

However, "I expect the CMIS dialog on center of the application" might be a good place to start looking into this for now.
Comment 7 Heiko Tietze 2020-10-17 12:26:27 UTC
>kwin_x11 -v
kwin 5.20.0
Comment 8 Michael Weghorn 2020-11-02 12:46:08 UTC
Created attachment 166935 [details]
screencast

(In reply to Heiko Tietze from comment #0)
> Follow-up to bug 128174: the CMIS dialog comes up centered of the main
> window at the first call and goes down to the lower right corner if shown
> again.

I can mostly reproduce this on debian testing, the window moves "further down" when the dialog is opened multiple times, s. attached screencast.

> The position goes even beyond the screen area making any interaction
> impossible.

This does not happen in my case, but may be dependent e.g. on the exact versions of kwin, kf5 or qt libraries.

Version: 7.1.0.0.alpha1+
Build ID: 0f3a8a972421aa440f4276b92463a481e5cd4267
CPU threads: 12; OS: Linux 5.9; UI render: default; VCL: kf5
Locale: en-GB (en_GB.UTF-8); UI: en-US
Calc: threaded
Comment 9 Michael Weghorn 2020-11-02 12:56:05 UTC
(In reply to Michael Weghorn from comment #6)
> However, "I expect the CMIS dialog on center of the application" might be a
> good place to start looking into this for now.

It seems that the dialog is supposed to remember and restore its previous state/position, handling added by this commit:

commit 7278462e12a6e41bb0e9496520e7a00d97856ce6
Author: Szymon Kłos 
Date:   Mon Aug 3 13:19:51 2015 +0200

    remember user settings
    
    Change-Id: I69c4672646365ca4cbeb04d6956741ffe365ad35


At a quick glance, it seems that for some reason

1) other VCL plugins don't restore the position at all

2) for qt5/kf5, there is some inconsistency between save and restore, so the position changes with each cycle (Qt5Frame::GetWindowState vs. Qt5Frame::SetWindowState)

@Heiko: I'd like to leave the question whether or not the state should generally be saved or not to you as UX expert (to possibly discuss with Szymon). Removing that handling makes the dialog open in the centre as desired.

The fact that qt5 restores the dialog at the wrong position looks wrong and I'll take a look at this (which IMHO makes sense in any case, independent of whether it's used for this specific dialog here or not).
Comment 10 Jan-Marek Glogowski 2020-11-02 13:11:58 UTC
Bug 130893 might be related too. That's probably also the reason for bug 134704 and eventually even bug 64438.

I have a patch, which works "most times" for dragging and docking, but not on the notebookbar case: https://gerrit.libreoffice.org/c/core/+/105181

IMHO the main problem is bug 130893, everything else a result of the hack with the native menu bar and wrong usage of maGeometry.n*Decoration, which AFAIK is used to describe the correct drawing area offsets inside a window. That is my understanding.
Comment 11 Michael Weghorn 2020-11-02 15:05:13 UTC
(In reply to Jan-Marek Glogowski from comment #10)
> Bug 130893 might be related too. That's probably also the reason for bug
> 134704 and eventually even bug 64438.
> 
> I have a patch, which works "most times" for dragging and docking, but not
> on the notebookbar case: https://gerrit.libreoffice.org/c/core/+/105181

How do you get floating toolbars for the notebookbar case? (I couldn't find out by quickly trying myself, so don't know where the exact issue is there)

> IMHO the main problem is bug 130893, everything else a result of the hack
> with the native menu bar and wrong usage of maGeometry.n*Decoration, which
> AFAIK is used to describe the correct drawing area offsets inside a window.
> That is my understanding.

Indeed, with https://gerrit.libreoffice.org/c/core/+/105181 (v1) applied, the mismatch becomes significantly smaller (i.e. the windows no more moves as far as before in one cycle, but it still doesn't remain in it's position, so there's probably another aspect for this case).
Comment 12 Michael Weghorn 2020-11-02 15:27:18 UTC
(In reply to Michael Weghorn from comment #11)
> Indeed, with https://gerrit.libreoffice.org/c/core/+/105181 (v1) applied,
> the mismatch becomes significantly smaller (i.e. the windows no more moves
> as far as before in one cycle, but it still doesn't remain in it's position,
> so there's probably another aspect for this case).

The other piece seems to be that that Qt5Frame::GetWindowState uses QWidget::geometry() (which excludes the window frame) while Qt5Frame::SetPosSize (which is called by Qt5Frame::SetWindowState) uses QWidget::move, which includes the window frame.

In a quick test with this modification on top of https://gerrit.libreoffice.org/c/core/+/105181 (v1), the remote files dialog generally opens at the same position again:

diff --git a/vcl/qt5/Qt5Frame.cxx b/vcl/qt5/Qt5Frame.cxx
index 551b5c03c1ff..5619aa2b60f1 100644
--- a/vcl/qt5/Qt5Frame.cxx
+++ b/vcl/qt5/Qt5Frame.cxx
@@ -678,7 +678,7 @@ bool Qt5Frame::GetWindowState(SalFrameState* pState)
     else
     {
         // geometry() is the drawable area, which is wanted here
-        QRect rect = scaledQRect(asChild()->geometry(), devicePixelRatioF());
+        QRect rect = scaledQRect(asChild()->frameGeometry(), devicePixelRatioF());
         pState->mnX = rect.x();
         pState->mnY = rect.y();
         pState->mnWidth = rect.width();


Note however, that the comment above the changed line explicitly states that 'geometry()' is wanted here!  I'd assume that using QWidget::setGeometry in Qt5Frame::SetPosSize instead should also have the same effect regarding this bug report (i.e. correct position restored), but I currently know too little about how all of this is supposed to work to be able to say whether that's reasonable without digging deeper into the whole topic.
Comment 13 Jan-Marek Glogowski 2020-11-02 15:37:15 UTC
The best start is probably to start from SalFrameGeometry and what values LO does with in them. My comment on it in SalFrame is what I figured should be correct. Documentation would be good to have too. Main problem is, that native menubar was just used by gtk and I tried to emulate its hack with lying about the correct geometry to get it somehow working.

If you want to fix it, expect to spend a bit more time on it in general.
Comment 14 Michael Weghorn 2020-11-04 16:19:38 UTC
(In reply to Jan-Marek Glogowski from comment #13)
> The best start is probably to start from SalFrameGeometry and what values LO
> does with in them. My comment on it in SalFrame is what I figured should be
> correct. Documentation would be good to have too. Main problem is, that
> native menubar was just used by gtk and I tried to emulate its hack with
> lying about the correct geometry to get it somehow working.
> 
> If you want to fix it, expect to spend a bit more time on it in general.

Thanks for all the hints, also in https://gerrit.libreoffice.org/c/core/+/105181 .
As of now, I don't think I'll get to look deeper into this anytime soon.

I just quickly checked what gtk3 does and it actually just didn't take the position into account at all when restoring the state, s.a. https://gerrit.libreoffice.org/c/core/+/105298 (which makes restoring position work in general, but not for the "Remote Files" dialog used here, since its position is still not retrieved correctly for some reason).

FWIW, other than all Linux VCL plugins, Windows does restore the original position of the "Remote Files" dialog correctly.
Comment 15 Michael Weghorn 2020-11-04 16:48:45 UTC
FWIW, the print dialog can also be used to reproduce this (but you need to actually confirm, e.g. print to PDF printer), otherwise it doesn't remember the last position at all.
Comment 16 Heiko Tietze 2020-11-05 08:54:54 UTC
(In reply to Michael Weghorn from comment #9)
> @Heiko: I'd like to leave the question whether or not the state should
> generally be saved or not to you as UX expert (to possibly discuss with
> Szymon). Removing that handling makes the dialog open in the centre as
> desired.

While most user settings should be store I don't see a good reason for dialogs in general. Would show the CMIS and similar dialogs always in parent's center.
Comment 17 Commit Notification 2022-06-05 10:18:26 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3f8d3fd4649ef09e86c735617383a4bda0425540

tdf#137471 Qt return frame pos + client area size

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2022-06-06 11:37:18 UTC
Jan-Marek Glogowski committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/ced641a34f8312a7fb93ee87bf6f13f30914da3a

tdf#137471 Qt return frame pos + client area size

It will be available in 7.3.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 Michael Weghorn 2022-06-10 07:22:11 UTC
With current master, the "Remote files" dialog still moves further downwards each time I open it.

However, when I apply https://gerrit.libreoffice.org/c/core/+/135082 (v9) on top, it works as expected, so that seems to be needed as well.

Version: 7.4.0.0.alpha1+ / LibreOffice Community
Build ID: 089c91b1ad16232f130cb50266732509f83c52eb
CPU threads: 12; OS: Linux 5.17; UI render: default; VCL: kf5 (cairo+xcb)
Locale: en-GB (en_GB.UTF-8); UI: en-US
Calc: threaded
Comment 20 Jan-Marek Glogowski 2022-06-10 11:53:50 UTC
(In reply to Michael Weghorn from comment #19)
> With current master, the "Remote files" dialog still moves further downwards
> each time I open it.
> 
> However, when I apply https://gerrit.libreoffice.org/c/core/+/135082 (v9) on
> top, it works as expected, so that seems to be needed as well.
> 
> Version: 7.4.0.0.alpha1+ / LibreOffice Community
> Build ID: 089c91b1ad16232f130cb50266732509f83c52eb
> CPU threads: 12; OS: Linux 5.17; UI render: default; VCL: kf5 (cairo+xcb)
> Locale: en-GB (en_GB.UTF-8); UI: en-US
> Calc: threaded

Hmm - I had blindly assumed the patch on it's own might fix the problem, after re-reading comment 12. So reopen until the main patch goes in.