Created attachment 511581 [details] [diff] [review] patch Until we fix bug 615483, we should at least make the "Learn More.." link alignment look a little more polished. Also, on Linux the use of padding instead of margin to align the link is resulting in ugly focus styles. Right now the themes treat this link differently, and this patch updates gnomestripe and pinstripe to do what winstripe does (although I had to add a -moz-margin-start: 0; to pinstripe to left-align the link correctly). I will attach screenshots of the changes.
Created attachment 511582 [details] osx changes
Comment on attachment 511581 [details] [diff] [review] patch Can you add comments that describe where the values come from? (match the toolkit notification button margin for winstripe, match toolkit button margin+padding for pinstripe, override default label margin to match description for pinstripe).
Created attachment 511588 [details] [diff] [review] patch for check-in
Comment on attachment 511583 [details] linux changes Shouldn't the link text be on the same line as the button label? Why does your change increase the button height?
Created attachment 511794 [details] [diff] [review] better patch So it seems the current .geolocation-text-link padding styles on linux are determining the height of the button because it is vertically filling its containing hbox. Using margin-top and margin-bottom, I was able to get the link aligned with the button label and keep the button height the same. On closer inspection I also realized that the link height on OSX was 1px off due to the button border, and I also realized that the link alignment is wrong in winstripe, so I updated my patch to include fix these. (I know this change is assuming the custom button styles are applied for all Windows themes, but bug 633400 will probably be fixed before bug 509642, and this is just supposed to be a small polish tweak for Firefox 4). I will attach screenshots of the changes.
Created attachment 511796 [details] screenshots
Comment on attachment 511794 [details] [diff] [review] better patch Can you just add align="center" to the container of the label and the button?
That would work for Linux, but I think other changes would be needed for Windows and OSX because there's a margin-top on the button. Perhaps that margin should really be on the hbox as well.
Sounds like the link could just have the same margin-top as the button, which would still simplify things.
But yes, giving the container a class and setting the margin is probably the best solution.
Created attachment 511853 [details] [diff] [review] even better patch This patch applies the margin to the hbox instead of its contents. I tested on OSX, Windows, and Linux with the geolocation, password, and add-ons notifications. I applied the margin to Linux even though it didn't exist before because I think the only reason it didn't exist is that we didn't get around to polishing the notifications on Linux. This will make the Linux notifications look at least a little closer to the mockups from bug 577927.
Created attachment 511854 [details] [diff] [review] even better patch Oops, some tabs got in there.
Comment on attachment 511854 [details] [diff] [review] even better patch I'd prefer popup-notification-button-container. The container doesn't really care about the button type.
Created attachment 511930 [details] [diff] [review] patch for check-in
Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre Verified issue and it's no longer present.