Closed Bug 515880 Opened 15 years ago Closed 15 years ago

Add drawintitlebar attribute to XUL root elements and sync it to the window widget

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [doc-waiting-1.9.3])

Attachments

(2 files, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
This is necessary for bug 513157.
Attachment #399963 - Flags: superreview?(roc)
Attachment #399963 - Flags: review?(neil)
I don't intend to land this before the Mac implementation is finished.
Looks good, but we need documentation of this new attribute as we discussed.
What's the intended use case here i.e. why a XUL attribute?
Attached image screenshot
Spec:
Setting drawintitlebar="true" on the root XUL element of a window extends the window's content area into the window frame. In the titlebar, the titlebar controls and the window title are drawn on top of the window content. The titlebar area may not respond to mouse events.
(In reply to comment #3)
> What's the intended use case here i.e. why a XUL attribute?

What else could it be? Would you prefer a CSS property?
Use cases are listed in bug 513157 comment 0.

If themes want to make use of this functionality, they can use CSS to add a binding that sets the attribute, like we used to do for (in)activetitlebarcolor in Firefox 3.0 (see globalBindings.xml).
Blocks: 520637
Attached patch v1.1 (obsolete) — Splinter Review
Updated to trunk. I noticed that I forgot to rev the nsIWidget IID; I'll do that when I land this.
Attachment #399963 - Attachment is obsolete: true
Attachment #404718 - Flags: superreview?(roc)
Attachment #404718 - Flags: review?(neil)
Attachment #399963 - Flags: superreview?(roc)
Attachment #399963 - Flags: review?(neil)
Comment on attachment 404718 [details] [diff] [review]
v1.1

+                                   NS_LITERAL_STRING("true").Equals(*aValue));

EqualsLiteral?
Attachment #404718 - Flags: superreview?(roc) → superreview+
Comment on attachment 404718 [details] [diff] [review]
v1.1

>         // Hide chrome if needed
[I wonder why we don't check for root content here too.]

>         if (aName == nsGkAtoms::hidechrome &&
>             mNodeInfo->Equals(nsGkAtoms::window) &&
>             aValue) {
>             HideWindowChrome(aValue && NS_LITERAL_STRING("true").Equals(*aValue));
[Heh, this checks aValue twice!]

>+  virtual void            SetDrawsInTitlebar(PRBool aState);
>   virtual PRBool          ShowsResizeIndicator(nsIntRect* aResizerRect);
>   virtual void            FreeNativeData(void * data, PRUint32 aDataType) {}
Could write virtual void SetDrawsInTitlebar(PRBool aState) {} perhaps?
Attachment #404718 - Flags: review?(neil) → review+
(In reply to comment #7)
> (From update of attachment 404718 [details] [diff] [review])
> +                                   NS_LITERAL_STRING("true").Equals(*aValue));
> 
> EqualsLiteral?
[Maybe aValue used to be an nsAString* and so ->EqualsLiteral wouldn't compile; it's an nsString* now so no such problem any more of course!]
Attached patch v2Splinter Review
(In reply to comment #7)
> (From update of attachment 404718 [details] [diff] [review])
> +                                   NS_LITERAL_STRING("true").Equals(*aValue));
> 
> EqualsLiteral?

OK.

(In reply to comment #8)
> (From update of attachment 404718 [details] [diff] [review])
> >         // Hide chrome if needed
> [I wonder why we don't check for root content here too.]

I wondered that too, but I don't want to change it in this patch.

> >         if (aName == nsGkAtoms::hidechrome &&
> >             mNodeInfo->Equals(nsGkAtoms::window) &&
> >             aValue) {
> >             HideWindowChrome(aValue && NS_LITERAL_STRING("true").Equals(*aValue));
> [Heh, this checks aValue twice!]

Fixed. Also changed this to EqualsLiteral.

> 
> >+  virtual void            SetDrawsInTitlebar(PRBool aState);
> >   virtual PRBool          ShowsResizeIndicator(nsIntRect* aResizerRect);
> >   virtual void            FreeNativeData(void * data, PRUint32 aDataType) {}
> Could write virtual void SetDrawsInTitlebar(PRBool aState) {} perhaps?

Sure.
Attachment #404718 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/ef1036610eb2
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Keywords: dev-doc-needed
Whiteboard: [doc-waiting-1.9.3]
This attribute definitely needs better documentation. I set it to true and expected the topmost element to magically jump up into the titlebar, but nothing happened. I know my approach is lame, but nothing I googled gave me a clue. A short usage example would be really helpful.
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.