Closed
Bug 869660
Opened 12 years ago
Closed 11 years ago
Pop-up windows on OSX are looking pretty strange
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: mconley, Assigned: mconley)
References
Details
(Whiteboard: [Australis:M4])
Attachments
(3 files, 2 obsolete files)
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.
Updated•12 years ago
|
Assignee | ||
Comment 1•12 years ago
|
||
A reference screenshot to work off of.
Assignee | ||
Comment 2•12 years ago
|
||
This seems to take care of the problem.
Dao, are there other variations I should care about besides chromehidden~="toolbar"?
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 4•12 years ago
|
||
(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.
Assignee | ||
Comment 5•12 years ago
|
||
Switched from height to padding.
Attachment #747088 -
Attachment is obsolete: true
Attachment #747088 -
Flags: review?(dao)
Assignee | ||
Updated•12 years ago
|
Attachment #747429 -
Flags: review?(dao)
Comment 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
(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+
Assignee | ||
Comment 8•12 years ago
|
||
Landed in UX as https://hg.mozilla.org/projects/ux/rev/81e58dc6d039
Whiteboard: [fixed-in-ux]
Updated•12 years ago
|
Whiteboard: [fixed-in-ux] → [Australis:M4][fixed-in-ux]
Comment 10•11 years ago
|
||
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.
Comment 11•11 years ago
|
||
(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•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 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.
Description
•