Open Bug 573973 Opened 9 years ago Updated 9 years ago

Also handle "drawintitlebar" attribute in nsXULWindow.cpp

Categories

(Core :: Widget, defect)

defect
Not set

Tracking

()

REOPENED
mozilla2.0b2

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(1 file)

Attached patch v1Splinter Review
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.
Attachment #453360 - Flags: review?(neil)
Attachment #453360 - Flags: review?(neil) → review+
http://hg.mozilla.org/mozilla-central/rev/ad7f9821597e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3b2
Depends on: 576394
Backed out because of bug 576394:
http://hg.mozilla.org/mozilla-central/rev/751b3e72a7eb
http://hg.mozilla.org/mozilla-central/rev/c173731c9d90
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Why don't we get rid of this and just use the chromemargin attribute?
Good point. I need to think about that.
(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.
(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
(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.
Ah, I see.

I've filed bug 576740 for the Mac implementation of SetNonClientMargins.
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).
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).
You need to log in before you can comment on or make changes to this bug.