Closed Bug 869660 Opened 12 years ago Closed 11 years ago

Pop-up windows on OSX are looking pretty strange

Categories

(Firefox :: Theme, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: mconley, Assigned: mconley)

References

Details

(Whiteboard: [Australis:M4])

Attachments

(3 files, 2 obsolete files)

Attached image Demonstration of problem (deleted) —
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.
Blocks: 625989
No longer blocks: australis-tabs-osx
Attached image What it should look like (deleted) —
A reference screenshot to work off of.
Attached patch Patch v1 (obsolete) (deleted) — Splinter Review
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.
Attached patch Patch v1.1 (obsolete) (deleted) — Splinter Review
Switched from height to padding.
Attachment #747088 - Attachment is obsolete: true
Attachment #747088 - Flags: review?(dao)
Attachment #747429 - 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+
Attached patch Patch v1.2 (r+'d by dao) (deleted) — Splinter Review
(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]
Whiteboard: [fixed-in-ux] → [Australis:M4][fixed-in-ux]
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.
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.

Attachment

General

Created:
Updated:
Size: