573973 - Also handle "drawintitlebar" attribute in nsXULWindow.cpp

Status: RESOLVED INVALID

(Core :: Widget, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

The drawintitlebar attribute is currently only handled in nsXULElement.cpp in AfterSetAttr and UnsetAttr. That works if you change the attribute after the window XUL has loaded, but it doesn't work if the attribute is present from the beginning.
We need to handle it in nsXULWindow.cpp, too. The "hidechrome" attribute is also handled in both places.

Comment 1

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/ad7f9821597e

Comment 2

[Markus Stange [:mstange]]

Backed out because of bug 576394:
http://hg.mozilla.org/mozilla-central/rev/751b3e72a7eb
http://hg.mozilla.org/mozilla-central/rev/c173731c9d90

Comment 3

[Jim Mathies [:jimm]]

Why don't we get rid of this and just use the chromemargin attribute?

Comment 4

[Markus Stange [:mstange]]

Good point. I need to think about that.

Comment 5

[Jim Mathies [:jimm]]

(In reply to comment #4)
> Good point. I need to think about that.

We're currently not using variable margins, so really a base implementation just provides the ability to turn margin on or off. drawintitlebar is hooked up so that it sets chrome margins to (0,-1,-1,-1) on windows. (0 = remove the margin, -1 leave the margin alone.) I think the drawintitlebar code in widget/mac could hook straight up to that without much effort.

Is the fx button enabled on mac currently? I was looking through browser and didn't see where it was set.

Comment 6

[Markus Stange [:mstange]]

(In reply to comment #5)
> We're currently not using variable margins

What does this mean? That the margins have to be either 0 or -1?

> I think the drawintitlebar code in
> widget/mac could hook straight up to that without much effort.

Yeah, shouldn't be too hard.

> Is the fx button enabled on mac currently? I was looking through browser and
> didn't see where it was set.

It's not, and as far as I know it won't be. It's wrapped in #ifdef MENUBAR_CAN_AUTOHIDE which is only set on Windows:
http://hg.mozilla.org/mozilla-central/diff/01c0e8955947/browser/base/Makefile.in

Comment 7

[Jim Mathies [:jimm]]

(In reply to comment #6)
> (In reply to comment #5)
> > We're currently not using variable margins
> 
> What does this mean? That the margins have to be either 0 or -1?

The code supports setting the value manually, but currently the browser only makes use of -1 (full margin) and 0 (no margin at all) on windows.

Comment 8

[Markus Stange [:mstange]]

Ah, I see.

I've filed bug 576740 for the Mac implementation of SetNonClientMargins.

Comment 9

[Alfred Kayser]

chromemargin values other than 0 and -1 do work, and then behaves more or less like a normal margin on the widget. 

It would be nicer if chromemargin can also be controlled from a theme (via css or such).

Comment 10

[Alfred Kayser]

Filed bug 590994 for the 'allow chromemargin:0,0,0,0 from a theme'.

Note, bug 576740 proposes to remove 'drawintitlebar' (as chromemargin can do more).

Comment 11

[Markus Stange [:mstange]]

drawintitlebar has been removed.