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

RESOLVED FIXED in Firefox 60

Status

()

defect
P2
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: stransky, Assigned: jhorak)

Tracking

(Blocks 1 bug)

Trunk
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 wontfix, firefox60 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Double-click on the drag area should maximize browser.
(Reporter)

Comment 1

a year ago
Posted patch drag.patch (obsolete) — Splinter Review
Patch by Jan Horak but seems to be broken with latest trunk.
(Reporter)

Updated

a year ago
Blocks: gtktitlebar
No longer blocks: 1399611
(Assignee)

Comment 2

a year ago
Dao, could you please help me out with this? I'm trying to figure out how it is done in Windows, what code actually handles double clicking on titlebar. I see that when double clicking the sizemode of <window> changes. I'm wondering where to actually call window.restore()/window.maximize() in the code for the Linux version or if it can be handled different way.

I would use a debugger in Windows, but that's not so easy with my Linux-only background.
Flags: needinfo?(dao+bmo)
(In reply to Jan Horak [:jhorak] from comment #2)
> Dao, could you please help me out with this? I'm trying to figure out how it
> is done in Windows, what code actually handles double clicking on titlebar.
> I see that when double clicking the sizemode of <window> changes. I'm
> wondering where to actually call window.restore()/window.maximize() in the
> code for the Linux version or if it can be handled different way.
> 
> I would use a debugger in Windows, but that's not so easy with my Linux-only
> background.

-moz-window-dragging: drag; takes care of this on Windows and Mac. Would be nice if our Gtk widget code implemented this too.
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 4

a year ago
(In reply to Dão Gottwald [::dao] from comment #3)
> (In reply to Jan Horak [:jhorak] from comment #2)
> > Dao, could you please help me out with this? I'm trying to figure out how it
> > is done in Windows, what code actually handles double clicking on titlebar.
> > I see that when double clicking the sizemode of <window> changes. I'm
> > wondering where to actually call window.restore()/window.maximize() in the
> > code for the Linux version or if it can be handled different way.
> > 
> > I would use a debugger in Windows, but that's not so easy with my Linux-only
> > background.
> 
> -moz-window-dragging: drag; takes care of this on Windows and Mac. Would be
> nice if our Gtk widget code implemented this too.

From a quick look it seems that windows implements

void nsWindow::UpdateWindowDraggingRegion(const LayoutDeviceIntRegion& aRegion)
{
}

IIRC it sets widget area which is "drag able" and widget can issue signal accordingly when clicked at:

https://dxr.mozilla.org/mozilla-central/rev/0bb0f14672fdda31c19aea1ed829e050d693b9af/widget/windows/nsWindow.cpp#5718
https://dxr.mozilla.org/mozilla-central/rev/0bb0f14672fdda31c19aea1ed829e050d693b9af/widget/windows/nsWindow.cpp#6462
(Assignee)

Comment 6

a year ago
Attaching basically the same patch. We find out with Martin, that double click event is actually handled by Windows itself (if we didn't overlook something obvious).

In nsWindow::ProcessMessage when the WM_NCHITTEST message is received the pointer location is tested if in HTCAPTION area and this information is returned as reply for the message.

Currently we don't have similar implementation exported in GTK.

https://searchfox.org/mozilla-central/rev/8839daefd69087d7ac2655b72790d3a25b6a815c/widget/windows/nsWindow.cpp#5398
Assignee: stransky → jhorak
Attachment #8928934 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8932072 - Flags: feedback?(dao+bmo)
(In reply to Jan Horak [:jhorak] from comment #6)
> Created attachment 8932072 [details] [diff] [review]
> toolbar-doubleclick-restore-maximize.patch
> 
> Attaching basically the same patch. We find out with Martin, that double
> click event is actually handled by Windows itself (if we didn't overlook
> something obvious).

Have you checked Mac too?
Flags: needinfo?(jhorak)
(Assignee)

Comment 8

a year ago
Looks like this code is handling double click on titlebar: 
https://searchfox.org/mozilla-central/rev/be78e6ea9b10b1f5b2b3b013f01d86e1062abb2b/widget/cocoa/nsChildView.mm#4596
So if reading it correctly, if the event is not processed by gecko (defaultPrevented is false) then if the doubleclick happened and in titlebar location, the the window is zoomed. I think we can do something similar in gtk.
Flags: needinfo?(jhorak)
Attachment #8932072 - Flags: feedback?(dao+bmo)
Comment hidden (mozreview-request)
(Reporter)

Comment 10

a year ago
mozreview-review
Comment on attachment 8952337 [details]
Bug 1417847 - [CSD] Doubleclick event on draggable area restores/maximizes window;

https://reviewboard.mozilla.org/r/221586/#review227428

::: widget/gtk/nsWindow.cpp:2814
(Diff revision 1)
>      gdouble pressure = 0;
>      gdk_event_get_axis ((GdkEvent*)aEvent, GDK_AXIS_PRESSURE, &pressure);
>      event.pressure = pressure ? pressure : mLastMotionPressure;
>  
> -    DispatchInputEvent(&event);
> +    bool defaultPrevented =
> +        (DispatchInputEvent(&event) == nsEventStatus_eConsumeNoDefault);

Please leave DispatchInputEvent() stand alone on the line.

::: widget/gtk/nsWindow.cpp:4421
(Diff revision 1)
>      }
>  
>      return mIsTransparent ? eTransparencyTransparent : eTransparencyOpaque;
>  }
>  
> +/**************************************************************

Please make the comment more subtle - remove the "*****************************" for instance.
(Reporter)

Comment 11

a year ago
mozreview-review
Comment on attachment 8952337 [details]
Bug 1417847 - [CSD] Doubleclick event on draggable area restores/maximizes window;

https://reviewboard.mozilla.org/r/221586/#review227430

::: widget/gtk/nsWindow.cpp:2822
(Diff revision 1)
> +    LayoutDeviceIntPoint pos = event.mRefPoint;
> +    if (!defaultPrevented
> +             && event.button == WidgetMouseEvent::eLeftButton
> +             && event.mClickCount == 2
> +             && mDraggableRegion.Contains(pos.x, pos.y)
> +             && mWindowType == eWindowType_toplevel)

no need to test toplevel window here as popup does not have the mDraggableRegion set.

::: widget/gtk/nsWindow.cpp:2823
(Diff revision 1)
> +    if (!defaultPrevented
> +             && event.button == WidgetMouseEvent::eLeftButton
> +             && event.mClickCount == 2
> +             && mDraggableRegion.Contains(pos.x, pos.y)
> +             && mWindowType == eWindowType_toplevel)
> +    {

nit: gecko style is brace "}" after ")"

::: widget/gtk/nsWindow.cpp:2825
(Diff revision 1)
> +             && event.mClickCount == 2
> +             && mDraggableRegion.Contains(pos.x, pos.y)
> +             && mWindowType == eWindowType_toplevel)
> +    {
> +        if (mSizeState == nsSizeMode_Maximized) {
> +          SetSizeMode(nsSizeMode_Normal);

nit: 4 space indent here.
Attachment #8952337 - Flags: review?(stransky)
Comment hidden (mozreview-request)
(Reporter)

Comment 13

a year ago
mozreview-review
Comment on attachment 8952337 [details]
Bug 1417847 - [CSD] Doubleclick event on draggable area restores/maximizes window;

https://reviewboard.mozilla.org/r/221586/#review227448
Attachment #8952337 - Flags: review?(stransky) → review+

Comment 14

a year ago
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/e7b57f8eeec3
[CSD] Doubleclick event on draggable area restores/maximizes window; r=stransky
https://hg.mozilla.org/mozilla-central/rev/e7b57f8eeec3
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60

Comment 16

a year ago
This doesn't seem to be fixed at all, unless I'm missing something. No matter what drag area I doubleclick on, whether it's in the empty unused space in the tab bar, or the 'flexible spaces' to the left and right of the URL field, it doesn't work.

I've tested in both Gnome 3.28 and KDE Plasma 5.12 and it doesn't work. Firefox Dev Edition 60.0 beta 12 (64-bit).

This is a really important thing for CSD apps to be able to do, as it really screws with muscle memory to not be able to doubleclick drag areas to maximize the window.

Comment 17

a year ago
Same here, doesn't work in Ubuntu 17.10 and Firefox Dev Edition 60.0 beta 12 (64-bit).
This is NOT fixed!
(Reporter)

Comment 18

a year ago
Okay I'll look at it.
(Reporter)

Comment 19

a year ago
Hm, maybe Ubuntu specific? I tested on Fedora 28 right now and it works fine.
It did not work when I just tried it in Ubuntu MATE 18.04 with the latest Nightly.
(Reporter)

Comment 21

a year ago
Hm, I wonder why it works for me, Ubuntu 17.10 / FF Developer edition. The double click maximize works on empty space in tab bar (by the tabs) but does not work on space by search/url bar.

Can you pleas try different theme? To switch to Default instead of the Dark which is used by Developer edition.
(Reporter)

Comment 22

a year ago
I can see that problem now but only on MATE. I can't reproduce that on gnome-shell.
(Reporter)

Comment 23

a year ago
Filed as Bug 1455235
You need to log in before you can comment on or make changes to this bug.