Closed Bug 888615 Opened 7 years ago Closed 6 years ago

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

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Whiteboard: [Australis:P-])

Attachments

(3 files)

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.
(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.
How? Do we not set chromemargin on popup windows on Windows?
(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? :-)
(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.
(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
(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?
Flags: needinfo?(mnoorenberghe+bmo)
(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.
Flags: needinfo?(mnoorenberghe+bmo)
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?
Flags: needinfo?(mstange)
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P?]
Whiteboard: [Australis:M?][Australis:P?] → [Australis:P-]
I think we might need this bug in order to properly fix bug 930094 and support lightweight themes.
Blocks: 930094
Would a XUL attribute work for that use case?
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] from comment #10)
> Would a XUL attribute work for that use case?

Yes, I believe it would!
This patch brings back window titles on all windows, indiscriminately.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #8345979 - Flags: review?(smichaud)
Attachment #8345979 - Flags: review?(roc)
Comment on attachment 8345979 [details] [diff] [review]
Part 2: Add handling of drawtitle attribute

Looks good to me.
Attachment #8345979 - Flags: review?(smichaud) → review+
Comment on attachment 8345980 [details] [diff] [review]
Part 3: Only draw title when [window wantsTitleDrawn]

Looks good to me.
Attachment #8345980 - Flags: review?(smichaud) → review+
You need to log in before you can comment on or make changes to this bug.