Last Comment Bug 708797 - The sync door hanger notification doesn't have a border when using classic or third-party themes
: The sync door hanger notification doesn't have a border when using classic or...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: Firefox 11
Assigned To: Joshua M [:soapy]
:
: Dão Gottwald [:dao]
Mentors:
Depends on:
Blocks: 618913
  Show dependency treegraph
 
Reported: 2011-12-08 12:49 PST by :Ehsan Akhgari
Modified: 2011-12-18 15:34 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (1.53 MB, image/png)
2011-12-08 12:49 PST, :Ehsan Akhgari
no flags Details
Screenshot (25.65 KB, image/png)
2011-12-08 14:22 PST, :Ehsan Akhgari
no flags Details
Possible patch (1.26 KB, patch)
2011-12-10 00:05 PST, Joshua M [:soapy]
no flags Details | Diff | Splinter Review
Possible patch v2 (1.38 KB, patch)
2011-12-10 22:28 PST, Joshua M [:soapy]
no flags Details | Diff | Splinter Review
Possible patch v3 (1.26 KB, patch)
2011-12-11 06:33 PST, Joshua M [:soapy]
no flags Details | Diff | Splinter Review
Possible patch v4 (1.34 KB, patch)
2011-12-17 08:27 PST, Joshua M [:soapy]
dao+bmo: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2011-12-08 12:49:40 PST
Created attachment 580155 [details]
Screenshot

See the screenshot.
Comment 1 Dão Gottwald [:dao] 2011-12-08 12:52:15 PST
Comment on attachment 580155 [details]
Screenshot

This appears to be a different screenshot.
Comment 2 :Ehsan Akhgari 2011-12-08 14:22:23 PST
Created attachment 580194 [details]
Screenshot

Sorry!
Comment 3 Joshua M [:soapy] 2011-12-10 00:05:09 PST
Created attachment 580627 [details] [diff] [review]
Possible patch

It appears the panel-promo-box has too much negative margins with Windows Classic theme and overlaps the border.
Comment 4 Dão Gottwald [:dao] 2011-12-10 11:05:08 PST
Comment on attachment 580627 [details] [diff] [review]
Possible patch

>+@media all and (-moz-windows-classic) {

This is backwards, you need to use -moz-windows-default-theme. See the panel[type="arrow"] styles in <http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/popup.css>.
Comment 5 Joshua M [:soapy] 2011-12-10 22:28:46 PST
Created attachment 580719 [details] [diff] [review]
Possible patch v2
Comment 6 Dão Gottwald [:dao] 2011-12-11 04:59:28 PST
Comment on attachment 580719 [details] [diff] [review]
Possible patch v2

Is this correct for Windows XP? Note that popup.css uses -moz-windows-default-theme not just for WINSTRIPE_AERO.
Comment 7 Joshua M [:soapy] 2011-12-11 05:35:49 PST
This bug also affects Windows Classic on Windows 7 as well as third-party themes. Unless you were referring to the styling of the 'panel-promo-box'? It seems it was styled like Windows 7 style on all themes, even third-party ones. Is that the intended behavior?
Comment 8 Dão Gottwald [:dao] 2011-12-11 05:41:08 PST
I was referring to panel-promo-box on Windows XP Luna, which I suspect attachment 580719 [details] [diff] [review] breaks.
Comment 9 Joshua M [:soapy] 2011-12-11 06:33:22 PST
Created attachment 580736 [details] [diff] [review]
Possible patch v3

Fixed styling on Windows XP Luna.
Comment 10 Dão Gottwald [:dao] 2011-12-16 19:56:42 PST
Comment on attachment 580736 [details] [diff] [review]
Possible patch v3

> .panel-promo-box {
>-  margin: 8px -16px -16px -16px;
>-  padding: 8px 16px;
>-  background-color: #f1f5fb;
>-  color: GrayText;
>+  margin: 8px -10px -10px;
>+  padding: 8px 10px;

This box is now transparent, so the padding plus the negative margin doesn't seem to make sense. How about:

  margin: 16px 0 -2px;

> %ifndef WINSTRIPE_AERO
>   border-bottom-left-radius: 6px;
>   border-bottom-right-radius: 6px;
> %endif

This should move inside the -moz-windows-default-theme block.
Comment 11 Joshua M [:soapy] 2011-12-17 08:27:35 PST
Created attachment 582542 [details] [diff] [review]
Possible patch v4

(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 580736 [details] [diff] [review]
> Possible patch v3
> 
> > .panel-promo-box {
> >-  margin: 8px -16px -16px -16px;
> >-  padding: 8px 16px;
> >-  background-color: #f1f5fb;
> >-  color: GrayText;
> >+  margin: 8px -10px -10px;
> >+  padding: 8px 10px;
> 
> This box is now transparent, so the padding plus the negative margin doesn't
> seem to make sense. How about:
> 
>   margin: 16px 0 -2px;
> 
> > %ifndef WINSTRIPE_AERO
> >   border-bottom-left-radius: 6px;
> >   border-bottom-right-radius: 6px;
> > %endif
> 
> This should move inside the -moz-windows-default-theme block.

Added requested changes.
Comment 13 Matt Brubeck (:mbrubeck) 2011-12-18 15:34:53 PST
https://hg.mozilla.org/mozilla-central/rev/e75f1076eab0

Note You need to log in before you can comment on or make changes to this bug.