1455235 - [CSD][MATE] Double-click on the drag area should maximize browser

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Martin Stránský [:stransky] (ni? me)]

+++ This bug was initially created as a clone of Bug #1417847 +++

Follow up from Bug 1417847.

Double-click on the drag area should maximize browser. It fails on MATE desktop.

Comment 1

[m.kurz]

Hi Martin,

this also doesn't work for me but I am NOT using the MATE desktop but GNOME-Flashback under Ubuntu 17.10.

This is my environment:

~$ env | grep "XDG_"
XDG_MENU_PREFIX=gnome-flashback-
XDG_GREETER_DATA_DIR=/var/lib/lightdm-data/mkurz
XDG_SESSION_TYPE=x11
XDG_DATA_DIRS=/usr/share/gnome-flashback-compiz:/usr/share/gnome-flashback-compiz:/usr/local/share:/usr/share:/var/lib/snapd/desktop:/var/lib/snapd/desktop
XDG_SESSION_DESKTOP=gnome-flashback-compiz
GNOME_SESSION_XDG_SESSION_PATH=/org/freedesktop/DisplayManager/Session0
XDG_SEAT_PATH=/org/freedesktop/DisplayManager/Seat0
XDG_CURRENT_DESKTOP=GNOME-Flashback:GNOME
XDG_RUNTIME_DIR=/run/user/1000
XDG_SESSION_PATH=/org/freedesktop/DisplayManager/Session0
XDG_CONFIG_DIRS=/etc/xdg/xdg-gnome-flashback-compiz:/etc/xdg/xdg-gnome-flashback-compiz:/etc/xdg

I also tried 
$MOZ_GTK_TITLEBAR_DECORATION=client ./firefox
and
$MOZ_GTK_TITLEBAR_DECORATION=system ./firefox

plus I tried using different themes.

Nothing did work.

Comment 2

[Jan Horak [:jhorak]]

Attachment: Bug 1455235 [gtk] Don't start moving window before mousemove event,

We need to start moving the window in GTK after mousemove event arrives,
not with mousedown, because the drag area can also process doubleclick
event to restore/maximize window. This also match to the GTK implementation,
see gedit behaviour for example.

Review commit: https://reviewboard.mozilla.org/r/237950/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237950/

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8969222 [details]
Bug 1455235 [gtk] Don't start moving window before mousemove event,

https://reviewboard.mozilla.org/r/237950/#review243660

::: toolkit/modules/WindowDraggingUtils.jsm:71
(Diff revision 1)
>            this._deltaX = aEvent.screenX - screenRect.left;
>            this._deltaY = aEvent.screenY - screenRect.top;
>          } else {
>            this._deltaX = aEvent.screenX - this._window.screenX;
>            this._deltaY = aEvent.screenY - this._window.screenY;
>          }

This code doesn't seem to make sense in the GTK case.

::: toolkit/modules/WindowDraggingUtils.jsm:78
(Diff revision 1)
>          this._window.addEventListener("mousemove", this);
>          this._window.addEventListener("mouseup", this);
>          break;
>        case "mousemove":
> +        if (/^gtk/i.test(AppConstants.MOZ_WIDGET_TOOLKIT)) {
> +            // On GTK, there is a toolkit-level function which handles

this is indented too far

::: toolkit/modules/WindowDraggingUtils.jsm:80
(Diff revision 1)
>          break;
>        case "mousemove":
> +        if (/^gtk/i.test(AppConstants.MOZ_WIDGET_TOOLKIT)) {
> +            // On GTK, there is a toolkit-level function which handles
> +            // window dragging, which must be used but we want to
> +            // start moving the window when the mousemove happens.

"On GTK, there is a toolkit-level function which handles window dragging. We want to start moving the window on the first mousemove event after mousedown."

Comment 4

[Jan Horak [:jhorak]]

Comment on attachment 8969222 [details]
Bug 1455235 [gtk] Don't start moving window before mousemove event,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237950/diff/1-2/

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8969222 [details]
Bug 1455235 [gtk] Don't start moving window before mousemove event,

https://reviewboard.mozilla.org/r/237950/#review244090

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc0d7de218cb
[gtk] Don't start moving window before mousemove event, r=dao

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/cc0d7de218cb

Comment 8

[Azari]

Hello, I'm the one who initially reported that doubleclick-to-maximize wasn't working in Bug 1417847, I just tested Nightly with the fix here, and it seems to work now.

One issue however is that it only works when doubleclicking the empty space in the tab bar, it doesn't work when doubleclicking the 'flexible spaces' that are in the navbar (to the left and right of the URL field, in default out of the box photon UI). In other platforms like Windows, doubleclicking those spaces is also supposed to trigger a maximize event.

Another thing I was wondering is whether these fixes would be uplifted to 60 or not?

Comment 9

[Martin Stránský [:stransky] (ni? me)]

(In reply to Azari from comment #8)
> Hello, I'm the one who initially reported that doubleclick-to-maximize
> wasn't working in Bug 1417847, I just tested Nightly with the fix here, and
> it seems to work now.

Great, thanks!

> One issue however is that it only works when doubleclicking the empty space
> in the tab bar, it doesn't work when doubleclicking the 'flexible spaces'
> that are in the navbar (to the left and right of the URL field, in default
> out of the box photon UI). In other platforms like Windows, doubleclicking
> those spaces is also supposed to trigger a maximize event.

Please file another bug for that.
 
> Another thing I was wondering is whether these fixes would be uplifted to 60
> or not?

I'll request uplift for this one.

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8969222 [details]
Bug 1455235 [gtk] Don't start moving window before mousemove event,

Approval Request Comment
[Feature/Bug causing the regression]: none
[User impact if declined]: Firefox main Window is not moved smoothly by mouse.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: Minor change in drag area handling code, Linux/Gtk+ only.
[String changes made/needed]: none

Comment 11

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8969222 [details]
Bug 1455235 [gtk] Don't start moving window before mousemove event,

Fix for minor issue for Linux users, let's uplift for beta 15.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/06f6ddcfe8d9

Comment 14

[Azari]

(In reply to Martin Stránský [:stransky] from comment #9)
> Please file another bug for that.

Okay, I've filed it as Bug 1457248. I added you and jhorak to the CC list, but I don't know if it notified you, so I'm commenting here just to be sure.