Closed Bug 1630085 Opened 4 years ago Closed 4 years ago

misuse _NET_WM_DESKTOP for session restore to virtual desktop

Categories

(Firefox :: Session Restore, defect)

75 Branch
All
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: wekerbugs, Unassigned)

References

(Regression)

Details

(Keywords: regression)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Call nsWindow::MoveToWorkspace() with an invalid workspaceIDStr. Say I currently have 1 virtual desktop and call it with "2".

Also expecting the same order after a restart of the WM. For example to restore the windows after a reboot.

Actual results:

nsWindow::MoveToWorkspace() without checking if the virtual desktop is valid requests to switch to the 3rd virtual desktop which is invalid by setting _NET_WM_DESKTOP to 2. This is not really defined by the freedesktop spec specifications.freedesktop.org.

There is also no real definition of order by the freedesktop spec which can be confusing especially for dynamically created desktops.

Expected results:

nsWindow::MoveToWorkspace() should check if the workspaceIDStr is valid before requesting.

Check if the order and count of the virtual desktops is the same after restarts.

Component: Untriaged → Session Restore
OS: Unspecified → Linux
Regressed by: 372650
Hardware: Unspecified → All
Has Regression Range: --- → yes
Keywords: regression

If I understand the spec correctly then nsWindow::MoveToWorkspace() also needs to check if it is allowed to change the desktop with _NET_WM_ALLOWED_ACTIONS testing for _NET_WM_ACTION_CHANGE_DESKTOP. However I could be wrong on this.

The atom _NET_NUMBER_OF_DESKTOPS and _NET_CURRENT_DESKTOP could also be helpfull.

(In reply to wekerbugs from comment #0)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Call nsWindow::MoveToWorkspace() with an invalid workspaceIDStr. Say I currently have 1 virtual desktop and call it with "2".

Session Restore only ever attempts to restore a window to the desktop it was in, we don't make them up. So whilst this is a theoretical possibility, the likelihood of this happening in practice is very slim.

Also expecting the same order after a restart of the WM. For example to restore the windows after a reboot.

nsWindow::MoveToWorkspace() without checking if the virtual desktop is valid requests to switch to the 3rd virtual desktop which is invalid by setting _NET_WM_DESKTOP to 2. This is not really defined by the freedesktop spec specifications.freedesktop.org.

There is also no real definition of order by the freedesktop spec which can be confusing especially for dynamically created desktops.

So this is also why we don't restrict this explicitly; Ubuntu Unity dynamically creates another desktop as soon as you move a window to the last one and removes it again once the desktop before it doesn't contain any windows anymore.
Therefore, in the restore case after (WM) restart, we need to optimistically move the window to a desktop that does not yet exist and rely on the Window Manager to handle the request correctly.
In practice, I've yet to find a case where this mechanism doesn't work. I'd love for you to report that if you do (under normal usage conditions).

Expected results:

nsWindow::MoveToWorkspace() should check if the workspaceIDStr is valid before requesting.

Check if the order and count of the virtual desktops is the same after restarts.

As I explained above, in order to support the wildly differing interpretations and implementations of the freedesktop.org spec, I prefer leaving it to the Window Manager to 'manage' possible restrictions here.

(In reply to wekerbugs from comment #1)

If I understand the spec correctly then nsWindow::MoveToWorkspace() also needs to check if it is allowed to change the desktop with _NET_WM_ALLOWED_ACTIONS testing for _NET_WM_ACTION_CHANGE_DESKTOP. However I could be wrong on this.

For these specifically I really think we should defer judgement and enforcement to the WM, not the client (Gecko, in this case). FWIW, I haven't seen any other clients do this, but I've only seen so many.

(In reply to wekerbugs from comment #2)

The atom _NET_NUMBER_OF_DESKTOPS and _NET_CURRENT_DESKTOP could also be helpfull.

You can check the nsWindow.cpp revisions to see that these were in fact used before, but purposefully removed!

If you don't mind, I'd like to mark this bug as invalid for now, but please feel free to re-open if you think I did that in error!

Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID

Session Restore only ever attempts to restore a window to the desktop it was in, we don't make them up. So whilst this is a theoretical possibility, the likelihood of this happening in practice is very slim.
Not in WMs like BSPWM where one has full controll over what virtual desktops are createt and in which order.

In practice, I've yet to find a case where this mechanism doesn't work. I'd love for you to report that if you do (under normal usage conditions).
Like I wrote BSPWM for example gives the freedom to create and destroy virtual desktops but does not do so automagically. If the atom is invalid BSPWM does strange things. I have lost some windows even.

A workaround would be Bug 1628742 for WMs that do not support automagically creating virtual desktops but support manually creating desktops.

I'm sorry for the formating of my last comment.

The strange things BSPWM does if the atom is invalid could very well be a bug in BSPWM not firefox.

You need to log in before you can comment on or make changes to this bug.