1446151 - Draw In Titlebar not working anymore on Trunk as of 2018-03-15

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Richard Marti (:Paenglab)]

No matter how the pref mail.tabs.drawInTitlebar is set, the window titlebar is always shown. Seen on Mac and Windows.

It looks like bug 1439875 is the reason for this.

maybe we need something like https://hg.mozilla.org/mozilla-central/rev/1ee5a2d21901#l1.1

Comment 1

[Emilio Cobos Álvarez (:emilio)]

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1445737, which is the same code looking fishy in FF. I'll try to address it pretty much that way.

Comment 2

[Jorg K (CEST = GMT+2)]

Bug 1439875 has just been backed out:
c43177719f36 Gurzau Raul — Backed out 6 changesets (bug 1439875) for massive breakage for nightly users a=backout

Comment 3

[Richard Marti (:Paenglab)]

Attachment: Bug1446151.patch

Tested on my Mac again with the build showing the problem. When I maximize the window the tabs are back in the titlebar. This shows, we initialize it at the wrong moment.

I made a port of bug 1445737 to be in sync. It doesn't fixes the issue on Mac but still works with the latest builds.

I tried to place TabsInTitlebar.onDOMContentLoaded(); on different places but didn't succeed. Maybe somebody can point to where this needs to be placed.

Comment 4

[Jorg K (CEST = GMT+2)]

Well, sadly M-C have refactored this area again:
https://hg.mozilla.org/mozilla-central/rev/e343c24ea4c0

So the TabsInTitlebar.onDOMContentLoaded() call is no longer there:
https://hg.mozilla.org/mozilla-central/rev/e343c24ea4c0#l3.53

Maybe you want to follow that.

TabsInTitlebar.init(); is done before they do new LightweightThemeConsumer():
https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/browser/base/content/browser.js#1208
so my suggestion would be to put the init() call and the whenWindowLayoutReady() into mailGlue:
https://dxr.mozilla.org/comm-central/rev/ab720c6d3a9f330ab95707c78337cf9b0449fbc7/mail/components/mailGlue.js#75

I might be wrong :-(

Comment 5

[Richard Marti (:Paenglab)]

Tried it and got a "TabsInTitlebar is not defined". Then copied the whole TabsInTitlebar function to mailGle.js and got "Window is not defined".

Maybe this should someone try that knows what he does. :(

Comment 6

[Jorg K (CEST = GMT+2)]

I was going to try it, but the patch here doesn't apply any more. I'm not sure how much of
https://hg.mozilla.org/mozilla-central/rev/e343c24ea4c0
we'd need to port.

In any case, something like this can be done:
+++ b/mail/components/mailGlue.js
           new LightweightThemeConsumer(aSubject.document);
+          let win = Services.wm.getMostRecentWindow("mail:3pane");
+          dump("=== Tabs in titlebar!!\n");
+          win.TabsInTitlebar.init();
         }

If you have TabsInTitlebar in msgMail3PaneWindow.js, then you can call it like this from mailGlue. You'd have to check with some logging whether that runs earlier than the call in OnLoadMessenger().

Sadly we don't have any expert on this unless you want to ask Florian who did the refactoring in bug 1448303.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: Bug1446151.patch

Refreshed and with dumps. You can see that the mailGlue code runs first, but it still doesn't work.

Florian, can you help us out here?

Comment 8

[Jorg K (CEST = GMT+2)]

There's been more happening in bug 1449689. We're really late following those changes.

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1446151-drawtabs-refactor.patch - WIP

Getting there. The problem is that at startup, 'allow' is set to false. Debugging that now.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1446151-drawtabs-refactor.patch (v2)

You wanted someone who knows what they're doing? Here you go.

This is refactored according to bug 1445737, bug 1448303 and bug 1449689 from browser-tabsintitlebar.js.

As far as I can see, everything is working on Windows.

I don't quite understand what that sizemode stuff is about. M-C have a handleEvent() function which doesn't appear to be called in the file. I rolled the code into the resize observer. Of course, `break` becomes `return`.

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1446151-drawtabs-refactor.patch (v2b)

Minor tweak: returned the variable name for the pref to the original.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1446151-drawtabs-refactor.patch (v3)

Now using handleEvent() to be even more in sync with M-C.

Comment 13

[Richard Marti (:Paenglab)]

Comment on attachment 8965230 [details] [diff] [review]
1446151-drawtabs-refactor.patch (v3)

Review of attachment 8965230 [details] [diff] [review]:
-----------------------------------------------------------------

Many thanks for fixing this.

I checked the FX file and our file with WinMerge and the differences are looking reasonable with our different implementations and features.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1446151-drawtabs-refactor.patch (v4)

Very good idea to use WinMerge to compare the two files. I found more things that should have been more similar, so that's fixed here now.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7bd736b50792
Refactor TabsInTitlebar according to bug 1445737, bug 1448613, bug 1448303 and bug 1449689. r=Paenglab