Closed Bug 1417847 Opened 7 years ago Closed 6 years ago

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

Categories

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

Product:
Core
Component:
Widget: Gtk
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: stransky, Assigned: jhorak)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

toolbar-doubleclick-restore-maximize.patch
2.33 KB, patch
Details | Diff | Splinter Review
Bug 1417847 - [CSD] Doubleclick event on draggable area restores/maximizes window;
59 bytes, text/x-review-board-request
stransky
: review+
Details 
Double-click on the drag area should maximize browser.
Attached patch drag.patch (obsolete) — — Splinter Review 
Patch by Jan Horak but seems to be broken with latest trunk.
Blocks: gtktitlebar
No longer blocks: 1399611
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)
(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
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)
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 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.
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 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+
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
Closed: 6 years ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
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.
Same here, doesn't work in Ubuntu 17.10 and Firefox Dev Edition 60.0 beta 12 (64-bit).
This is NOT fixed!
Okay I'll look at it.
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.
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.
I can see that problem now but only on MATE. I can't reproduce that on gnome-shell.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: