Closed Bug 577928 Opened 9 years ago Closed 9 years ago

7/Vista specific styling of menu buttons in popup notifications

Categories

(Toolkit :: Themes, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: Terepin, Assigned: Margaret)

References

Details

Attachments

(2 files, 16 obsolete files)

55.04 KB, image/png
Details
7.70 KB, patch
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; sk; rv:2.0b2pre) Gecko/20100710 Minefield/4.0b2pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; sk; rv:2.0b2pre) Gecko/20100710 Minefield/4.0b2pre

Doorhanger is using Jumplists-like style.

Reproducible: Always
Blocks: 577927
Version: unspecified → Trunk
Summary: Windows specific styling of Doorhanger → 7/Vista (Aero Glass) specific styling of Doorhanger
Attached patch wip (obsolete) — Splinter Review
Assignee: nobody → margaret.leibovic
Attachment #479912 - Flags: feedback?(dao)
Attached image wip screenshot (obsolete) —
Attachment #479913 - Flags: feedback?(shorlander)
Attached image wip screenshot (pressed state) (obsolete) —
Attachment #479915 - Flags: feedback?(shorlander)
Attached patch patch (obsolete) — Splinter Review
Dao, do you think I should factor out these button styles to a shared.inc file? If so, we can update the button styles in the edit bookmark panel. I can do that in a follow-up bug if we want.
Attachment #479912 - Attachment is obsolete: true
Attachment #480276 - Flags: review?(dao)
Attachment #479912 - Flags: feedback?(dao)
Attached image patch screenshot (obsolete) —
Attachment #479913 - Attachment is obsolete: true
Attachment #479915 - Attachment is obsolete: true
Attachment #479913 - Flags: feedback?(shorlander)
Attachment #479915 - Flags: feedback?(shorlander)
Comment on attachment 480276 [details] [diff] [review]
patch

The bug summary seems to indicate that you should do this for aero glass (and basic, I suppose), but this patch applies the style everywhere (e.g. XP luna, classic, random third-party themes).
Attachment #480276 - Flags: review?(dao) → review-
(In reply to comment #6)
> (and basic, I suppose)

Hmm, is this patch part of edge styling, or it only styles the pop-up itself?
I also made the dropmarker side of the button a little wider to better match the mock-ups.
Attachment #480276 - Attachment is obsolete: true
Attachment #480707 - Flags: review?(dao)
Whiteboard: [needs review dao]
Attached patch patch v3 (obsolete) — Splinter Review
Updated patch to deal with https://bugzilla.mozilla.org/show_bug.cgi?id=554937#c51.
Attachment #480707 - Attachment is obsolete: true
Attachment #483297 - Flags: review?(dao)
Attachment #480707 - Flags: review?(dao)
Attached image patch v3 screenshot
Attachment #480277 - Attachment is obsolete: true
Comment on attachment 483297 [details] [diff] [review]
patch v3

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css
>@@ -1855,31 +1855,32 @@ toolbarbutton.bookmark-item[dragover="tr
>   background-color: #fffcd6;
>   border: 1px solid #dad8b6;
>   padding: 5px 5px 5px 5px;
>   font-weight: bold;
> }
> 
> /* Notification popup */
> #notification-popup {
>-  padding: 10px;
>+  margin-left: -17px;
>+  margin-right: -17px;
> }

This belongs in toolkit.
Attachment #483297 - Flags: review?(dao) → review-
Couldn't DH use reall Glass efect instead of fake one?
Fwiw, I think the code (popup.xml#arrowpanel) that deals with the positioning should really deal with doing it right (iow, decide exactly where to position the panel based on the position attribute or argument passed to openPopup). Setting the position with margins and css is a) hacky and b) not consistent with how panels are supposed to work.

If we just want to enforce one type of positioning for arrow panels (which actually may make perfect sense) then I guess it would be fine to do it this way.
(In reply to comment #12)
> Couldn't DH use reall Glass efect instead of fake one?

A little correction: Aero Glass contains these three efects:
1. Transparency
2. Blur
3. Stripes
This implementation is using only transparency. What about the rest two?
(In reply to comment #13)
> Fwiw, I think the code (popup.xml#arrowpanel) that deals with the positioning
> should really deal with doing it right (iow, decide exactly where to position
> the panel based on the position attribute or argument passed to openPopup).
> Setting the position with margins and css is a) hacky and b) not consistent
> with how panels are supposed to work.
> 
> If we just want to enforce one type of positioning for arrow panels (which
> actually may make perfect sense) then I guess it would be fine to do it this
> way.

If we put the positioning css in toolkit, won't it fix the issue for all arrow panels? The styles in popup.css and popup-aero.css are already doing arrow panel positioning, so I don't think it would be too hacky to put the fix in there.
They position the arrow on the panel, not the panel itself. If just repositioning the arrow itself (without touching the actual panel) is enough, then yeah, the css is perfect. I just assumed the panel itself would need repositioning, but this makes more sense actually.

So what will end up happening (and what happens now, only a little off) is that the panel will continue to appear wherever the position attibute (or argument) specifies and the arrow will adjust according to the panel position to be directly pointed at the anchorNode. That would be the best option IMO.
Attached patch patch v4 (obsolete) — Splinter Review
Moved the css to toolkit. I just added it to popup-aero.css, since the positioning for the non-aero themes can be resolved in the different theme bugs.
Attachment #483297 - Attachment is obsolete: true
Attachment #483640 - Flags: review?(dao)
Summary: 7/Vista (Aero Glass) specific styling of Doorhanger → 7/Vista specific styling of Doorhanger
Comment on attachment 483640 [details] [diff] [review]
patch v4

>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css

>+  .popup-notification-menubutton > .button-menubutton-button,
>+  .popup-notification-menubutton > .button-menubutton-dropmarker {
>+    -moz-appearance: none;
>+    margin: 0;
>+    background-image: -moz-linear-gradient(top, rgba(250,250,250,.6), rgba(175,175,175,.25) 49%, rgba(0,0,0,.12) 51%, rgba(0,0,0,.09) 60%, rgba(0,0,0,.05));
>+    box-shadow: 0 0 1px 1px rgba(255,255,255,.75) inset;
>+    -moz-border-end: 1px solid rgba(0,0,0,.35);
>+  }
>+
>+  .popup-notification-menubutton > .button-menubutton-button {
>+    padding: 0;
>+    -moz-padding-start: 8px;
>+    -moz-padding-end: 5px;
>+    -moz-border-end: 1px solid rgba(0,0,0,.35);
>+  }

-moz-border-end: 1px solid rgba(0,0,0,.35); is duplicated in the above blocks -- I assume only the latter is wanted?

>+  .popup-notification-menubutton > .button-menubutton-button:hover,
>+  .popup-notification-menubutton > .button-menubutton-dropmarker:hover {
>+    background-image: -moz-linear-gradient(top, rgba(250,250,250,.9), rgba(200,200,200,.6) 49%, rgba(0,0,0,.23) 51%, rgba(0,0,0,.17) 60%, rgba(0,0,0,.05));
>+    box-shadow: 0 0 0 1px rgba(255,255,255,1) inset,
>+                0 0 2px 1px rgba(255,255,255,.75) inset;

nit: use 'white' instead of 'rgba(255,255,255,1)'

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

> .geolocation-text-link {
>-  padding-top: 5px;
>+  margin-top: 17px;
> }

Shouldn't the buttons margin be 17px as well regardless of the Vista/7 theme?
Comment on attachment 483640 [details] [diff] [review]
patch v4

>--- a/toolkit/themes/winstripe/global/popup-aero.css
>+++ b/toolkit/themes/winstripe/global/popup-aero.css
>@@ -62,16 +62,18 @@ menupopup > menu > menupopup {
>   margin-top: -3px;
> }
> 
> panel[type="arrow"] {
>   -moz-appearance: none;
>   background: transparent;
>   border: none;
>   -moz-transition: opacity 300ms;
>+  margin-left: -17px;
>+  margin-right: -17px;
> }
> 
> .panel-arrowcontent {
>   border-radius: 6px;
>   background-color: rgba(179,230,255,.35);
>   background-image: -moz-linear-gradient(rgba(250,253,255,.9), rgba(250,253,255,.75) 20px, rgba(250,253,255,.70) 39px, rgba(250,253,255,0) 41px, rgba(250,253,255,0));
>   margin: 5px;
>   box-shadow: 0 0 0 1px rgba(255,255,255,.65) inset,
>@@ -85,17 +87,17 @@ panel[type="arrow"] {
>   background-clip: padding-box;
>   margin: 4px;
>   border: 2px solid;
>   -moz-border-radius: 2px;
>   -moz-border-top-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
>   -moz-border-left-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
>   -moz-border-bottom-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
>   -moz-border-right-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
>-  padding: 6px;
>+  padding: 14px;
> }
> 
> .panel-arrow[side="top"] {
>   list-style-image: url("chrome://global/skin/arrow/panelarrow-up.png");
>   margin-bottom: -5px;
> }
> 
> .panel-arrow[side="bottom"] {

Is this really Aero specific?

Does margin-left: -17px; margin-right: -17px; work for RTL? How does it behave if there's not enough room for the popup at the right or bottom?
blocking2.0: --- → ?
(In reply to comment #19)
> Comment on attachment 483640 [details] [diff] [review]
> patch v4
> 
> >--- a/toolkit/themes/winstripe/global/popup-aero.css
> >+++ b/toolkit/themes/winstripe/global/popup-aero.css
> >@@ -62,16 +62,18 @@ menupopup > menu > menupopup {
> >   margin-top: -3px;
> > }
> > 
> > panel[type="arrow"] {
> >   -moz-appearance: none;
> >   background: transparent;
> >   border: none;
> >   -moz-transition: opacity 300ms;
> >+  margin-left: -17px;
> >+  margin-right: -17px;
> > }

This probably isn't aero-specific, since the same thing fixed the OSX positioning. I just assumed we would fix the positioning for non-aero arrow panels in bug 590072. Are styles in popup.css picked up in aero builds? I thought I had tried changing that file and didn't notice the changes in my build.

> >@@ -85,17 +87,17 @@ panel[type="arrow"] {
> >   background-clip: padding-box;
> >   margin: 4px;
> >   border: 2px solid;
> >   -moz-border-radius: 2px;
> >   -moz-border-top-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
> >   -moz-border-left-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
> >   -moz-border-bottom-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
> >   -moz-border-right-colors: rgba(255,255,255,.6) rgba(0,0,0,.7);
> >-  padding: 6px;
> >+  padding: 14px;
> > }

This is aero specific because non-aero styles don't have .panel-inner-arrowcontent styles, since they don't have the glass border.

> > .panel-arrow[side="top"] {
> >   list-style-image: url("chrome://global/skin/arrow/panelarrow-up.png");

> Does margin-left: -17px; margin-right: -17px; work for RTL? How does it behave
> if there's not enough room for the popup at the right or bottom?

It works for RTL and when there's not enough room for the popup at the bottom, but there are problems when there isn't enough room for the popup at the right. I will work on fixing this now.
(In reply to comment #20)
> It works for RTL and when there's not enough room for the popup at the bottom,
> but there are problems when there isn't enough room for the popup at the right.
> I will work on fixing this now.

The margin fix works properly when applied to the non-aero theme, and I'm having trouble figuring out what's causing the problem on the aero themes. I assume it has to do with the fact that there are different margins on the arrow and content elements in the aero theme.

Enn, do you have any thoughts about how to fix this?
Actually, I just kept removing styles from my patch to see what changed the behavior, and I eventually removed my patch altogether, but the horizontal positioning of the doorhanger is still different than in the nightlies. I think my build may have somehow become broken, so I will rebuild and hopefully the problem will go away.
(In reply to comment #22)
> Actually, I just kept removing styles from my patch to see what changed the
> behavior, and I eventually removed my patch altogether, but the horizontal
> positioning of the doorhanger is still different than in the nightlies. I think
> my build may have somehow become broken, so I will rebuild and hopefully the
> problem will go away.

I don't know what went wrong, but a clobber build fixed the problem.

Dão, the positioning works correctly with this patch. However, the patch will have to be updated when bug 605332 lands (thanks for filing that and writing a patch, btw!).
Attached patch patch v5 (obsolete) — Splinter Review
Updated to apply after bug 605332 landed.
Attachment #483640 - Attachment is obsolete: true
Attachment #484719 - Flags: review?(dao)
Attachment #483640 - Flags: review?(dao)
Comment on attachment 484719 [details] [diff] [review]
patch v5

You seem to have missed comment 18.
Attachment #484719 - Flags: review?(dao) → review-
Attached patch patch v6 (obsolete) — Splinter Review
Sorry I missed those comments! Should be addressed now.
Attachment #484719 - Attachment is obsolete: true
Attachment #485072 - Flags: review?(dao)
Comment on attachment 485072 [details] [diff] [review]
patch v6

> panel[type="arrow"] {
>   -moz-appearance: none;
>   background: transparent;
>   border: none;
>   -moz-transition: opacity 300ms;
>+  margin-left: -17px;
>+  margin-right: -17px;
> }

This will break arrow popups that appear to the left or right of the anchor.

What are you trying to accomplish here? Are you trying to move the arrow so that it appears to point to something? Why 17 pixels? 17 pixels may be suitable for the notification popup but would be wrong for other popups, where the anchor is much larger. It seems that this should belong in browser.css or somesuch.

In the long run it would be better if the arrow popup determined the margins itself based on the position and size of the arrow and its image, such that the arrow was always centered on the anchor. But this can be a different bug.
(In reply to comment #27)
> What are you trying to accomplish here? Are you trying to move the arrow so
> that it appears to point to something? Why 17 pixels? 17 pixels may be suitable
> for the notification popup but would be wrong for other popups, where the
> anchor is much larger. It seems that this should belong in browser.css or
> somesuch.

Yes, we want the arrow to point the the icon that anchors it. Right now it's misaligned (https://bugzilla.mozilla.org/show_bug.cgi?id=554937#c51). I dont't know how the arrow panels behave for other types of notifications, since they're not used elsewhere right now, but I agree that this fix probably won't generalize well. So maybe we should just move the fix to browser.css for now and apply it only to #notification-popup? Dão, what do you think?

> In the long run it would be better if the arrow popup determined the margins
> itself based on the position and size of the arrow and its image, such that the
> arrow was always centered on the anchor. But this can be a different bug.

Yes, I agree. There should probably be a follow-up to bug 554937 for this.
I noticed that test_arrowpanel.xul is failing with the current patch. I tried programatically setting the margin in popup.xul (in this method http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/popup.xml#298), but the tests still fail with that margin. I think it has to do with the use of the panel bounding box to check its position. I propose sorting this out in another bug.

The tests don't fail if we set the margin on #noficiation-popup in browser.css, so maybe we should just do that for this patch until we get an arrowpanel fix.
Can we keep this bug focused on just the button?
Attached patch patch v7 (obsolete) — Splinter Review
Here's a patch that makes no attempt to fix the positioning of the arrow panel. I will file a new bug for that.
Attachment #485072 - Attachment is obsolete: true
Attachment #485163 - Flags: review?(dao)
Attachment #485072 - Flags: review?(dao)
(In reply to comment #31)
> Created attachment 485163 [details] [diff] [review]
> patch v7
> 
> Here's a patch that makes no attempt to fix the positioning of the arrow panel.
> I will file a new bug for that.

Filed bug 606343.
Comment on attachment 485163 [details] [diff] [review]
patch v7

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css
>@@ -1828,32 +1828,32 @@ toolbarbutton.bookmark-item[dragover="tr
>   -moz-appearance: none;
>   background-color: #fffcd6;
>   border: 1px solid #dad8b6;
>   padding: 5px 5px 5px 5px;
>   font-weight: bold;
> }
> 
> /* Notification popup */
>-#notification-popup {
>-  padding: 10px;
>-}

This change seems to cut off the shadow at least on XP. Not sure why this code exists here in the first place, though. Looks like it belongs in popup.css...
(In reply to comment #33)
> Comment on attachment 485163 [details] [diff] [review]
> patch v7
> 
> >--- a/browser/themes/winstripe/browser/browser.css
> >+++ b/browser/themes/winstripe/browser/browser.css
> >@@ -1828,32 +1828,32 @@ toolbarbutton.bookmark-item[dragover="tr
> >   -moz-appearance: none;
> >   background-color: #fffcd6;
> >   border: 1px solid #dad8b6;
> >   padding: 5px 5px 5px 5px;
> >   font-weight: bold;
> > }
> > 
> > /* Notification popup */
> >-#notification-popup {
> >-  padding: 10px;
> >-}
> 
> This change seems to cut off the shadow at least on XP. Not sure why this code
> exists here in the first place, though. Looks like it belongs in popup.css...

I can't see any difference on Windows 7, except for the fact that the padding moves the arrow panel farther away from the anchor.

Adding it to popup.css causes test_arrowpanel.xul to fail :(
(In reply to comment #34)
> Adding it to popup.css causes test_arrowpanel.xul to fail :(

That doesn't sound like a good reason to keep it out of popup.css but to fix the test ;-)
Attached image xp screenshot (obsolete) —
This is what I see with the padding removed on XP.
(In reply to comment #35)
> (In reply to comment #34)
> > Adding it to popup.css causes test_arrowpanel.xul to fail :(
> 
> That doesn't sound like a good reason to keep it out of popup.css but to fix
> the test ;-)

I'm having a tough time figuring out what I would need to fix in the test. I think the problem may be rooted in the popupshowing handler popup.xml. Can we just leave the padding in browser.css for now, then fix it when we fix bug 606343?
(In reply to comment #37)
> Can we just leave the padding in browser.css for now, then fix it when we
> fix bug 606343?

Yes.
Comment on attachment 485163 [details] [diff] [review]
patch v7

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+.popup-notification-menubutton {
>+  margin-top: 17px;
>+}

It looks like this belongs in toolkit. I don't think we have a popup-notification specific stylesheet, so maybe global.css.

I'm not sure whether the browser-aero.css changes belong there as well -- depends on whether we expect that other apps will want the same button styling.
> >+.popup-notification-menubutton {
> >+  margin-top: 17px;
> >+}
> 
> It looks like this belongs in toolkit. I don't think we have a
> popup-notification specific stylesheet, so maybe global.css.

I don't see how such a large margin would apply to all notifications or all menu buttons.
If it doesn't apply to notifications in general then it shouldn't be applied to all notifications in browser.xul either, as opposed to what the current patch is doing.
I meant to say 'all arrow panels', not 'all notifications'. For instance, the bookmarks panel or a similar more complex UI might be in a arrow panel, and a large margin on any menu-buttons would be inappropriate there.

I'm not familiar enough with what is possible with the notifications (those created with the PopupNotification script), but I could imagine the margin would apply to all of those.
popup-notification-menubutton is a notification-specific class, so it wouldn't apply to all arrow panels.
OK, I misunderstood when you mentioned putting this into global.css.
(In reply to comment #39)
> Comment on attachment 485163 [details] [diff] [review]
> patch v7
> 
> >--- a/browser/themes/winstripe/browser/browser.css
> >+++ b/browser/themes/winstripe/browser/browser.css
> 
> >+.popup-notification-menubutton {
> >+  margin-top: 17px;
> >+}
> 
> It looks like this belongs in toolkit. I don't think we have a
> popup-notification specific stylesheet, so maybe global.css.
> 
> I'm not sure whether the browser-aero.css changes belong there as well --
> depends on whether we expect that other apps will want the same button styling.

If we're factoring some styles out to toolkit in this patch, should I just factor out the button styles as well? I filed bug 601719 about this, but now that arrow panels landed I can just take care of it here. I feel like other apps would want button styling that fits in with the rest of the arrow panel styling, since arrow panels do have their own style.
(In reply to comment #45)
> (In reply to comment #39)
> > Comment on attachment 485163 [details] [diff] [review] [details]
> > patch v7
> > 
> > >--- a/browser/themes/winstripe/browser/browser.css
> > >+++ b/browser/themes/winstripe/browser/browser.css
> > 
> > >+.popup-notification-menubutton {
> > >+  margin-top: 17px;
> > >+}
> > 
> > It looks like this belongs in toolkit. I don't think we have a
> > popup-notification specific stylesheet, so maybe global.css.
> > 
> > I'm not sure whether the browser-aero.css changes belong there as well --
> > depends on whether we expect that other apps will want the same button styling.
> 
> If we're factoring some styles out to toolkit in this patch, should I just
> factor out the button styles as well? I filed bug 601719 about this, but now
> that arrow panels landed I can just take care of it here. I feel like other
> apps would want button styling that fits in with the rest of the arrow panel
> styling, since arrow panels do have their own style.

Yes, the button styling is what I meant when I said "the browser-aero.css changes".
(In reply to comment #39)
> Comment on attachment 485163 [details] [diff] [review]
> patch v7
> 
> >--- a/browser/themes/winstripe/browser/browser.css
> >+++ b/browser/themes/winstripe/browser/browser.css
> 
> >+.popup-notification-menubutton {
> >+  margin-top: 17px;
> >+}
> 
> It looks like this belongs in toolkit. I don't think we have a
> popup-notification specific stylesheet, so maybe global.css.

What about notification.css?
We could use notification.css for the popup-notification binding, yes.
(In reply to comment #47)
> What about notification.css?

Probably a good idea, even more so if Firefox wants to go in the direction of bug 595810 at some point as well.
Attachment #485163 - Flags: review?(dao)
Attached patch simple implementation (obsolete) — Splinter Review
This patch simply sets the 'side' on the panel which can be used to adjust the margin.
Assignee: margaret.leibovic → enndeakin
Status: NEW → ASSIGNED
(In reply to comment #50)
> Created attachment 487369 [details] [diff] [review]
> simple implementation
> 
> This patch simply sets the 'side' on the panel which can be used to adjust the
> margin.

I think this patch belongs in bug 606343. I think we decided we were just going to work on the styling of the inside of the notifications in this bug, mainly the button styling.
Attached patch patch v8 (obsolete) — Splinter Review
I moved the button styling into toolkit. I also got rid of any styling that changes the position of the panel, since that will be worked on in bug 606343.
Assignee: enndeakin → margaret.leibovic
Attachment #485163 - Attachment is obsolete: true
Attachment #487400 - Flags: review?(dao)
> I think this patch belongs in bug 606343.

Bah, that's what I had intended to do.
You seem to have lost the windows-default-theme dependency when moving the code to toolkit.
Summary: 7/Vista specific styling of Doorhanger → 7/Vista specific styling of menu buttons in popup notifications
blocking2.0: ? → betaN+
Attached patch patch v9 (obsolete) — Splinter Review
Is this how I should be incorporating -moz-windows-default-theme? I was thinking we would want to use some of the button styles for the non-aero patch since I believe their shape and size should be the same, but I don't know how that would work if we need to declare the aero styles inside the default theme selector. I guess that can just be a problem for the patch in bug 590072?
Attachment #487400 - Attachment is obsolete: true
Attachment #487613 - Flags: review?(dao)
Attachment #487400 - Flags: review?(dao)
Attachment #487369 - Attachment is obsolete: true
Attachment #485173 - Attachment is obsolete: true
Comment on attachment 487613 [details] [diff] [review]
patch v9

>+  .popup-notification-menubutton {
>+    -moz-appearance: none;
>+    border: 1px solid rgba(0,0,0,.35);
>+    border-radius: 3px;
>+    padding: 0;
>+  }
>+
>+  .popup-notification-menubutton:hover:active {
>+    border: 1px solid rgba(0,0,0,.5);

nit:
border-color: rgba(0,0,0,.5);
Attachment #487613 - Flags: review?(dao) → review+
Attached patch final patch (obsolete) — Splinter Review
Attachment #487613 - Attachment is obsolete: true
Whiteboard: [needs review dao] → [can land]
I forgot a "*" in the jar file.
Attachment #487654 - Attachment is obsolete: true
Blocks: 590072
Component: Theme → Themes
Product: Firefox → Toolkit
QA Contact: theme → themes
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Sigh. You added notification-areo.css with DOS line endings, which breaks cross-compiles, since the host python won't strip the CR and ends up trying to include "notification.css\r" instead.
(In reply to comment #60)
> Sigh. You added notification-areo.css with DOS line endings, which breaks
> cross-compiles, since the host python won't strip the CR and ends up trying to
> include "notification.css\r" instead.

fixed:
http://hg.mozilla.org/mozilla-central/rev/10ff849f3fdb
This looks good, except the hover effect doesn't work on the dropmarker. It only does on the left part of the button. This isn't caused by faulty css, but somehow the hover doesn't register. This might be bug 390984 ? I can't open the xul file to see if it is a similar problem. I guess I should file a new bug on this, since it isn't caused by bad styling.


btw, this line does seem to have no function (since the dropdown menu is always open when it is in this state):
.popup-notification-menubutton > .button-menubutton-dropmarker:hover:active,
Depends on: 610123
Depends on: 620896
You need to log in before you can comment on or make changes to this bug.