Also handle "drawintitlebar" attribute in nsXULWindow.cpp

REOPENED
Assigned to

Status

()

Core
Widget
REOPENED
8 years ago
8 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
mozilla2.0b2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

v1
902 bytes, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
Created attachment 453360 [details] [diff] [review]
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.
Attachment #453360 - Flags: review?(neil)

Updated

8 years ago
Attachment #453360 - Flags: review?(neil) → review+
(Assignee)

Comment 1

8 years ago
http://hg.mozilla.org/mozilla-central/rev/ad7f9821597e
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3b2
(Assignee)

Updated

8 years ago
Depends on: 576394
(Assignee)

Comment 2

8 years ago
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 → ---

Comment 3

8 years ago
Why don't we get rid of this and just use the chromemargin attribute?
(Assignee)

Comment 4

8 years ago
Good point. I need to think about that.

Comment 5

8 years ago
(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.
(Assignee)

Comment 6

8 years ago
(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

8 years ago
(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.
(Assignee)

Comment 8

8 years ago
Ah, I see.

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

Comment 9

8 years ago
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

8 years ago
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.