888615 - Decide when we want to draw the window title and expose a proper API

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Markus Stange [:mstange]]

Before bug 676241, all windows showed the window title.
After bug 676241, drawintitlebar windows no longer had a window title.
And now, after bug 877767, we're drawing the window title on all windows again - at least on mozilla-inbound (and soon mozilla-central), not sure what to do on ux.

With Australis, we must not show the window title for regular windows. But we do want it for popup windows (which don't have a tabbar), correct?
Maybe the simple thing to do would be to make popup windows not be drawintitlebar windows any more. However, in private browsing mode, we probably still want to have the purple badge in the titlebar for popup windows, right? Not sure if we currently do that on mozilla-central. We don't seem to have the badge in private browsing popup windows on UX.

How do we handle this on Windows? Do popup windows have a window title there?

In any case, we should have a proper API for turning off window title drawing.

Comment 1

[Dão Gottwald [:dao]]

(In reply to Markus Stange [:mstange] from comment #0)
> How do we handle this on Windows? Do popup windows have a window title there?

Yep, pre-Australis and with it too.

Comment 2

[Markus Stange [:mstange]]

How? Do we not set chromemargin on popup windows on Windows?

Comment 3

[:Gijs (out for now; he/him)]

(In reply to Markus Stange [:mstange] from comment #2)
> How? Do we not set chromemargin on popup windows on Windows?

If we're showing the title etc., then yes, we remove the chromemargin attribute. See: 
http://hg.mozilla.org/projects/ux/file/adb9531aead0/browser/base/content/browser.js#l4528

Your patches from bug 877767 just landed on central. Should we merge everything to UX, or only part of it? :-)

Comment 4

[Markus Stange [:mstange]]

(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Markus Stange [:mstange] from comment #2)
> > How? Do we not set chromemargin on popup windows on Windows?
> 
> If we're showing the title etc., then yes, we remove the chromemargin
> attribute. See: 
> http://hg.mozilla.org/projects/ux/file/adb9531aead0/browser/base/content/
> browser.js#l4528

Interesting. So private browsing popup windows don't have a badge in the titlebar on Windows?

> Your patches from bug 877767 just landed on central. Should we merge
> everything to UX, or only part of it? :-)

I'd recommend backing out the second changeset on UX after the merge, until we've decided what to do about it.

Comment 5

[:Gijs (out for now; he/him)]

(In reply to Markus Stange [:mstange] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > (In reply to Markus Stange [:mstange] from comment #2)
> > > How? Do we not set chromemargin on popup windows on Windows?
> > 
> > If we're showing the title etc., then yes, we remove the chromemargin
> > attribute. See: 
> > http://hg.mozilla.org/projects/ux/file/adb9531aead0/browser/base/content/
> > browser.js#l4528
> 
> Interesting. So private browsing popup windows don't have a badge in the
> titlebar on Windows?

Possibly; I can't check very quickly right now, sorry.


> > Your patches from bug 877767 just landed on central. Should we merge
> > everything to UX, or only part of it? :-)
> 
> I'd recommend backing out the second changeset on UX after the merge, until
> we've decided what to do about it.

https://hg.mozilla.org/projects/ux/rev/82c63da57b32

Comment 6

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Markus Stange [:mstange] from comment #4)
> > (In reply to :Gijs Kruitbosch from comment #3)
> > > (In reply to Markus Stange [:mstange] from comment #2)
> > > > How? Do we not set chromemargin on popup windows on Windows?
> > > 
> > > If we're showing the title etc., then yes, we remove the chromemargin
> > > attribute. See: 
> > > http://hg.mozilla.org/projects/ux/file/adb9531aead0/browser/base/content/
> > > browser.js#l4528
> > 
> > Interesting. So private browsing popup windows don't have a badge in the
> > titlebar on Windows?


As far as I can tell, UX's private browsing windows on Windows don't seem to have any indicator at all at the moment. Faaaaairly sure that's a bug. Matt, any idea what caused that?

Comment 7

[Matthew N. [:MattN]]

(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to :Gijs Kruitbosch from comment #5)
> > > Interesting. So private browsing popup windows don't have a badge in the
> > > titlebar on Windows?
> 
> As far as I can tell, UX's private browsing windows on Windows don't seem to
> have any indicator at all at the moment. Faaaaairly sure that's a bug. Matt,
> any idea what caused that?

That's bug 868640.

Comment 8

[Justin Dolske [:Dolske]]

I'm not sure what this bug is about. Is something currently broken? Or is this just an optimization? What's the impact if we shipped as-is?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

I think we might need this bug in order to properly fix bug 930094 and support lightweight themes.

Comment 10

[Markus Stange [:mstange]]

Would a XUL attribute work for that use case?

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Markus Stange [:mstange] from comment #10)
> Would a XUL attribute work for that use case?

Yes, I believe it would!

Comment 12

[Markus Stange [:mstange]]

Attachment: Part 1: bring back the patch for bug 877767 on Australis

This patch brings back window titles on all windows, indiscriminately.

Comment 13

[Markus Stange [:mstange]]

Attachment: Part 2: Add handling of drawtitle attribute

Comment 14

[Markus Stange [:mstange]]

Attachment: Part 3: Only draw title when [window wantsTitleDrawn]

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8345979 [details] [diff] [review]
Part 2: Add handling of drawtitle attribute

Looks good to me.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8345980 [details] [diff] [review]
Part 3: Only draw title when [window wantsTitleDrawn]

Looks good to me.

Comment 17

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae31fa8d7f7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/2dff63fed312
https://hg.mozilla.org/integration/mozilla-inbound/rev/2250a49996a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/555a5f56c847
https://hg.mozilla.org/integration/mozilla-inbound/rev/0abc0b17f9ce

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ae31fa8d7f7f
https://hg.mozilla.org/mozilla-central/rev/2dff63fed312
https://hg.mozilla.org/mozilla-central/rev/2250a49996a2
https://hg.mozilla.org/mozilla-central/rev/555a5f56c847
https://hg.mozilla.org/mozilla-central/rev/0abc0b17f9ce