1161565 - Clicking the titlebar adds a new tab instead of maximizing

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Firefox Nightly on Yosemite

STR:
Click an empty area in the titlebar on the right half of the browser screen.

Expected result:
Application maximizes

Actual Result:
A new tab is opened

The latter result happens even though the click was nowhere near the new tab button, nor was its visual affordance activated. With some experimenting, it appears that you can manage to maximize Firefox, if you hit the upper few pixels of the titlebar area. There's no visual indication which part that is, though, in fact the entire titlebar and the area to the right of the tabs is visually identical.

Tested Safari and Chrome: they work correctly and maximize.

Comment 1

[Markus Stange [:mstange]]

We could remove support for the new tab double click shortcut when tabs are in the titlebar. Phillipp, what do you think?

Comment 2

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

I think the behavior that double-clicking the title bar maximizes the window is new in Yosemite, right?

I'm fine with removing the functionality there and landing it to see what feedback is like.

FWIW, all browsers on Windows (including Firefox) maximize when double-clicking the title bar.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

I wouldn't know, I've never used a Mac before - which is why it stood out clearly that Firefox behaves differently from other Mac apps.

>FWIW, all browsers on Windows (including Firefox) maximize when double-clicking the title bar

Confirmed.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

My first attempt was to remove the special condition in the dblclick handler in tabbrowser.xml. With this change, double clicking the tabbar doesn't open new tab anymore, but it doesn't maximize the window either.

Then I figured out whether to do maximizing is handled in ChildView:mouseUp. There is a check:
> (locationInTitlebar < [(ToolbarWindow*)[self window] titlebarHeight] ||
>  locationInTitlebar < [(ToolbarWindow*)[self window] unifiedToolbarHeight])
I guess the unifiedToolbarHeight one should be true here for clicking on the tabbar.

Then I investigated in this path a bit, and found this height is set by nsChildView::UpdateThemeGeometries(). This method first computes height of the titlebar (via looking for theme geometry with type nsNativeThemeCocoa::eThemeGeometryTypeTitlebar), then finds all toolbars whose y is higher than the bottom of the titlebar.

It should work just fine. However, UpdateThemeGeometries() cannot find any titlebar at all on Yosemite and later, because the titlebar has "-moz-mac-vibrancy-light" instead of "-moz-window-titlebar" as the value of "-moz-appearance" since bug 1052466.

Backing-out that bug and then removing the special condition mentioned at the beginning can fix this bug. Anyone interested could try this build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a703b9aa9e02


If we really want to keep the effect from that bug, we probably should try to apply that to "-moz-window-titlebar" as well, instead of changing its appearance property, because as shown here, "-moz-window-titlebar" itself is a functional value which could affect other settings.

But I suggest we just backout that bug, and stop applying that effect to titlebar anymore, because that isn't actually an OS X convention. Some of the builtin applications do have translucent titlebar/toolbar, however, the stuff under their titlebar/toolbar is the content from the app itself (e.g. page in Safari), not the something behind the window. We would be able to use the identical effect in the future after we implement backdrop-filter (bug 1178765), but before that happens, I don't think we should have such half-done effect.

Thoughts?

Comment 6

[Markus Stange [:mstange]]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #5)
> But I suggest we just backout that bug, and stop applying that effect to
> titlebar anymore, because that isn't actually an OS X convention. [...]
> I don't think we should have such half-done effect.

I agree. I argued the same thing in bug 1052466 comment 10 and couldn't get a clear answer.

Comment 7

[:Gijs (he/him)]

What to do with the titlebar in terms of appearance isn't a question I can answer. -> :phlsa

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

Attachment: screenshot of the current style

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

Attachment: screenshot after the proposed change

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

Please see the two screenshots.

Personally I think the style after the proposed change is more consistent with the overall style of Firefox, and also follows the system convention better.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch

Michael Maslaney and Philipp Sackl have both said "OK" to the UI change in private emails. Let's do it!

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

https://hg.mozilla.org/integration/fx-team/rev/f8becc4132146dcd14b63858b1539f28e2ed6961
Bug 1161565 - Give titlebar proper appearance on OS X Yosemite and revmoe special case of double click behavior on unified tabbar on OS X. r=gijs

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

FWIW, talos reports a 7.12% improvement on one test by this change:

Improvement: Fx-Team - tscroll-ASAP - MacOSX 10.10 (e10s) - 7.12% decrease
--------------------------------------------------------------------------
    Previous: avg 7.251 stddev 0.195 of 12 runs up to revision 7603d2860165a1add86793a77f159cf68b2299a4
    New     : avg 6.734 stddev 0.135 of 12 runs since revision f8becc4132146dcd14b63858b1539f28e2ed6961
    Change  : -0.517 (7.12% / z=2.643)
    Graph   : http://mzl.la/1Q6dhiW

Changeset range: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=7603d2860165a1add86793a77f159cf68b2299a4&tochange=f8becc4132146dcd14b63858b1539f28e2ed6961

Comment 15

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8670117 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: probably not a regression
[User impact if declined]: hard to get the browser window maximized on Yosemite like other app
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: doesn't seem to be risky because it just changes some special cases back to what we do in general
[String/UUID change made/needed]: n/a

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

I'm on Yosemite 10.10.5 and can't reproduce this in Aurora 43. Xidorn, did you see it there? 
Double clicking the title bar minimizes my windows, rather than maximizing.

Comment 17

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16)
> I'm on Yosemite 10.10.5 and can't reproduce this in Aurora 43. Xidorn, did
> you see it there? 

I'm also on 10.10.5, and can reproduce this in Aurora 43. I guess you cannot reproduce it because you use some addon which disables browser.tabs.drawInTitlebar (like Tree Style Tab), so that the tabbar is not unified with the titlebar.

> Double clicking the title bar minimizes my windows, rather than maximizing.

It's a weird result...

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Nope, fresh Aurora profile, no add-ons. (Though I do love tree style tab in my usual profile!) 

But, I have the “Double-click a window’s title bar to minimize” option set in the Mac Dock preferences. When I uncheck that option, I'm getting a new tab whenever I double click in the Firefox title bar.

Comment 19

[Xidorn Quan [:xidorn] UTC+11]

Given that this bug require us to revert bug 1052466, and that bug was landed on Firefox 36, all versions up to ESR are affected.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8670117 [details] [diff] [review]
patch

Fix for user-facing feature, has been on m-c a few days. 
Approved for uplift to aurora.

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e9d716c60d37

Comment 22

[Xidorn Quan [:xidorn] UTC+11]

FWIW, again, talos reports a 13.8% improvement on one test by several changes including this one:

Improvement: Mozilla-Aurora - TP5 Scroll - MacOSX 10.10 - 13.8% decrease
------------------------------------------------------------------------
    Previous: avg 7.316 stddev 0.367 of 12 runs up to revision 68f9590115499ee32320d3268a6365ef8b248ba4
    New     : avg 6.308 stddev 0.164 of 12 runs since revision 2fbc5d0584e4797a21b232a26f95f0fc3bceca04
    Change  : -1.008 (13.8% / z=2.747)
    Graph   : http://mzl.la/1G5o4dg

Changeset range: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=68f9590115499ee32320d3268a6365ef8b248ba4&tochange=2fbc5d0584e4797a21b232a26f95f0fc3bceca04

Changesets:
  * http://hg.mozilla.org/releases/mozilla-aurora/rev/3902adefa47c
    : Ehsan Akhgari <[e-mail]> - Bug 1212683 - Fix and enable update.https.html; r=jdm, a=test-only

The source of the missing python script is:
https://chromium.googlesource.com/chromium/src/+/8a0f7fc/third_party/WebKit/LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/update-worker.php
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1212683

  * http://hg.mozilla.org/releases/mozilla-aurora/rev/e9d716c60d37
    : Xidorn Quan <[e-mail]> - Bug 1161565 - Give titlebar proper appearance on OS X Yosemite and revmoe special case of double click behavior on unified tabbar on OS X. r=gijs, a=lizzard
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1161565

  * http://hg.mozilla.org/releases/mozilla-aurora/rev/228866eb07ab
    : Francois Marier <[e-mail]> - Bug 1208629 - Properly support data: and blob: URIs with an integrity atribute. r=ckerschb, a=lizzard
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1208629

  * http://hg.mozilla.org/releases/mozilla-aurora/rev/2fbc5d0584e4
    : Xidorn Quan <[e-mail]> - Bug 1211344 - Stop setting various window attributes when they are not in 'persist' attribute. r=enndeakin, a=lizzard
    : http://bugzilla.mozilla.org/show_bug.cgi?id=1211344

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

Confirming the fix on Mac OSX 10.10 using Firefox 43 RC build 1 (build ID 20151208100201) and latest Aurora 44 (build ID 20151213004008).

Comment 24

[(inactive) Nihanth Subramanya [:nhnt11]]

I've been using the double-click-for-new-tab functionality for a very very long time (with tabs in the titlebar). It'd have been nice if a pref could have been added for this at least. Maybe that's still a good idea for another bug?