Pop-up windows on OSX are looking pretty strange

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

unspecified
Firefox 28
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M4])

Attachments

(3 attachments, 2 obsolete attachments)

Created attachment 746634 [details]
Demonstration of problem

This was noticed using a recent build of UX after bug 865374. We probably shouldn't be drawing in the titlebar for these windows. See screenshot.
Created attachment 747071 [details]
What it should look like

A reference screenshot to work off of.
Created attachment 747088 [details] [diff] [review]
Patch v1

This seems to take care of the problem.

Dao, are there other variations I should care about besides chromehidden~="toolbar"?
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Attachment #747088 - Flags: review?(dao)
Comment on attachment 747088 [details] [diff] [review]
Patch v1

>--- a/browser/components/customizableui/content/toolbar.xml	Tue May 07 10:33:08 2013 -0400
>+++ b/browser/components/customizableui/content/toolbar.xml	Wed May 08 15:52:35 2013 -0400

>   <binding id="toolbar">
>+    <resources>
>+      <stylesheet src="chrome://global/skin/toolbar.css"/>
>+    </resources>

What does this have to do with this bug?

>--- a/browser/themes/osx/browser.css	Tue May 07 10:33:08 2013 -0400
>+++ b/browser/themes/osx/browser.css	Wed May 08 15:52:35 2013 -0400

>+#main-window[chromehidden~="toolbar"] > #titlebar {
>+  height: 22px;
>+}
>+
>+#main-window:not([chromehidden~="toolbar"]) > #titlebar {
>   padding-top: 9px;
> }

Can't you use padding-top in both cases?

> Dao, are there other variations I should care about besides
> chromehidden~="toolbar"?

No, this should be sufficient as of bug 855370.
(In reply to Dão Gottwald [:dao] from comment #3)
> Comment on attachment 747088 [details] [diff] [review]
> Patch v1
> 
> >--- a/browser/components/customizableui/content/toolbar.xml	Tue May 07 10:33:08 2013 -0400
> >+++ b/browser/components/customizableui/content/toolbar.xml	Wed May 08 15:52:35 2013 -0400
> 
> >   <binding id="toolbar">
> >+    <resources>
> >+      <stylesheet src="chrome://global/skin/toolbar.css"/>
> >+    </resources>
> 
> What does this have to do with this bug?
> 

We need this rule: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/osx/global/toolbar.css#7

Otherwise the popup "toolbar" that contains the urlbar has a white background. Did you have an alternative suggestion?

> >--- a/browser/themes/osx/browser.css	Tue May 07 10:33:08 2013 -0400
> >+++ b/browser/themes/osx/browser.css	Wed May 08 15:52:35 2013 -0400
> 
> >+#main-window[chromehidden~="toolbar"] > #titlebar {
> >+  height: 22px;
> >+}
> >+
> >+#main-window:not([chromehidden~="toolbar"]) > #titlebar {
> >   padding-top: 9px;
> > }
> 
> Can't you use padding-top in both cases?
> 

Sure, can do.

> > Dao, are there other variations I should care about besides
> > chromehidden~="toolbar"?
> 
> No, this should be sufficient as of bug 855370.
Created attachment 747429 [details] [diff] [review]
Patch v1.1

Switched from height to padding.
Attachment #747088 - Attachment is obsolete: true
Attachment #747088 - Flags: review?(dao)
Comment on attachment 747429 [details] [diff] [review]
Patch v1.1

>-#titlebar {
>+#main-window[chromehidden~="toolbar"] > #titlebar {
>+  padding-top: 22px;
>+}
>+
>+#main-window:not([chromehidden~="toolbar"]) > #titlebar {
>   padding-top: 9px;
> }

write it like this instead:

#titlebar {
  padding-top: 9px;
}

#main-window[chromehidden~="toolbar"] > #titlebar {
  padding-top: 22px;
}
Attachment #747429 - Flags: review?(dao) → review+
Created attachment 747471 [details] [diff] [review]
Patch v1.2 (r+'d by dao)

(In reply to Dão Gottwald [:dao] from comment #6)
> Comment on attachment 747429 [details] [diff] [review]
> Patch v1.1
> 
> >-#titlebar {
> >+#main-window[chromehidden~="toolbar"] > #titlebar {
> >+  padding-top: 22px;
> >+}
> >+
> >+#main-window:not([chromehidden~="toolbar"]) > #titlebar {
> >   padding-top: 9px;
> > }
> 
> write it like this instead:
> 
> #titlebar {
>   padding-top: 9px;
> }
> 
> #main-window[chromehidden~="toolbar"] > #titlebar {
>   padding-top: 22px;
> }

Ah, yes, that's much better / clearer. Thank you!
Attachment #747429 - Attachment is obsolete: true
Attachment #747471 - Flags: review+
Whiteboard: [fixed-in-ux] → [Australis:M4][fixed-in-ux]

Updated

5 years ago
Duplicate of this bug: 888022
Has this fix started being built in the nightlies? I'm seeing broken popup titlebar behavior in the most recent (2013/7/8) build of the UX branch.
(In reply to Chenxia Liu [:liuche] from comment #10)
> Has this fix started being built in the nightlies? I'm seeing broken popup
> titlebar behavior in the most recent (2013/7/8) build of the UX branch.

Yes, it should have been there two months ago. I suppose you should re-open bug 888022 then.

Comment 12

5 years ago
https://hg.mozilla.org/mozilla-central/rev/81e58dc6d039
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M4][fixed-in-ux] → [Australis:M4]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.