+++ This bug was initially created as a clone of Bug #722758 +++ I'm rarely using Composer, thus I didn't notice that the same problem applies here as well (solved for Mail & News before): - open the Composer window (new or existing profile) - make sure that the toolbar has the "Print" button somewhere - this looks fine while using the Default theme - switch to Modern theme, now it is too low and misaligned Seen in urrent Windows 7 and Linux builds.

Great, Ian's workaround for Mail & News does it directly for Composer as well! This takes the additions from bug 722758 attachment 593257 [details] [diff] [review] and puts them into the spot where bug 676991 attachment 553967 [details] [diff] [review] removed the old printButton code. BTW: I've double-check the Address Book toolbar, but it doesn't have a Print button in its toolbar palette and thus isn't affected.

It appears that the Composer mail toolbar doesn't have any small toolbar icons, which would simplify the rules quite a bit. I'm posting this as an alternative version, thus please r+ either one or the other. If you want to ensure that this won't regress again if and when Composer gets a customizable toolbar, then attachment 722967 [details] [diff] [review] would be the better choice.

(In reply to rsx11m from comment #2) > BTW: I've double-check the Address Book toolbar, but it doesn't have a Print > button in its toolbar palette and thus isn't affected. Well, I've failed to look into the Mail/News Message Composition toolbar, which shows the same issue as the other toolbars despite not having been touched by the change in bug 676991. Thus, I'll amend the patch to that window as well. I've also added a dependency to bug 606683 on customization of the Composer main toolbar as part of that bug. Right now I tend to go with the simpler code for the editor/editorPrimaryToolbar.css fix, given that it will have to be revisited when completing the Composer toolbar customization with small icons anyway.

This is attachment 722984 [details] [diff] [review] for the Composer plus code for the Message Composition window. The latter needs the "long" version given that the toolbar is already fully configurable.

This may be too close to the next scheduled merge (in about to weeks) to consider the fix for 2.17b, but I'd definitely try to get it in for 2.18a2 once approved.

Comment on attachment 725888 [details] [diff] [review] Proposed fix (v2) > +/* To workaround the composer icons are 33px tall */ The grammar looks a bit off. How about: "To workaround that the composer icons are all 33px tall" > +/* To workaround the mailnews icons are 33px tall and have no small versions */ --ditto-- +++ b/suite/themes/modern/messenger/messengercompose/messengercompose.css +toolbar[iconsize="small"] > toolbarpaletteitem > #print-button, +toolbar[iconsize="small"] > #print-button { + list-style-image: url("chrome://communicator/skin/icons/common.png"); +} + +toolbar[iconsize="small"] > toolbarpaletteitem > #print-button, +toolbar[iconsize="small"] > #print-button, +#print-button { + -moz-image-region: rect(5px 42px 38px 0); +} You can consolidate the above two rules: toolbar[iconsize="small"] > toolbarpaletteitem > #print-button, toolbar[iconsize="small"] > #print-button, #print-button { list-style-image: url("chrome://communicator/skin/icons/common.png"); -moz-image-region: rect(5px 42px 38px 0); } r=me with the above fixed.

(In reply to Philip Chee from comment #7) > > +/* To workaround the composer icons are 33px tall */ > The grammar looks a bit off. How about: > "To workaround that the composer icons are all 33px tall" Verbatim copy of Ian's comment from bug 722758 (except for "composer") thus I assumed that it's correct even though it sounded funny to me as well. ;-) > You can consolidate the above two rules: I wasn't sure about that due to the extra #print selector for the second rule, but it works fine with both large and small settings in all states. Both fixed, ready to be pushed on trunk.

Comment on attachment 728629 [details] [diff] [review] Final patch (v3) [c-c: comment #10] (In reply to rsx11m from comment #6) > This may be too close to the next scheduled merge (in about to weeks) to > consider the fix for 2.17b, but I'd definitely try to get it in for 2.18a2 > once approved. I don't know if I have to effectively wait until the patch is checked in before being able to request approval, but I'm doing so now thus you can do so as you see fit and possibly check it in on all channels at the same time. If I'm not mistaken, there should be one beta left next week, thus trying for comm-beta as well unless it's considered too close to the release by now. [Approval Request Comment] Regression caused by (bug #): bug 676991 User impact if declined: misalignment in modern theme, no impact on function Testing completed (on m-c, etc.): c-c and c-a on Windows and Linux Risk to taking this patch (and alternatives if risky): none anticipated String changes made by this patch: none

Pushed to comm-central http://hg.mozilla.org/comm-central/rev/b5f136f16770 Thanks rsx11m

Phil, thanks for checking in the main patch. Meanwhile, I've also modified primaryToolbar.css according to your comment #7, adjusting the comment and consolidating the "small" rules. In this way, both versions are identical. Here the quick follow-up patch in case you want this for Ian's version too. I'm not envisioning this for the aurora/beta channel as the existing code works fine for those, so this would be for trunk only.

I just remembered why we did http://hg.mozilla.org/comm-central/rev/b5f136f16770#l2.13 in that order of rules: 2.13 +toolbar[iconsize="small"] > toolbarpaletteitem > #print-button, 2.14 +toolbar[iconsize="small"] > #print-button, 2.15 +#print-button { Which was to minimize churn in Hg Blame. However we are writing new styles here so we might as well have rearranged the order to: #print-button, toolbar[iconsize="small"] > #print-button, toolbar[iconsize="small"] > toolbarpaletteitem > #print-button { ... And: #print-button:hover, toolbar[iconsize="small"] > #print-button:hover { ... ...etc... Sorry for not spotting this earlier. > Meanwhile, I've also modified primaryToolbar.css according to your comment #7 Since it's working prefectly well now I'd leave it until there's need to rewrite. This code would need a rewrite anyway if/when we get small icons working.

Ok, so we don't want attachment 728725 [details] [diff] [review] if I understand you correctly, but do you want me to post a new follow-up patch relative to comm-central changeset b5f136f16770 addressing comment #12 on the order of the rules? Now I'm a bit confused...

(frankly, I'd prefer to have the primary-toolbar and the composition-toolbar versions not to differ too much as they'd likely be addressed at the same time when small toolbar buttons are coming, thus flipping the order in messagecompose compared to the primary toolbar may be rather confusing in this regard.)

Ok, so here comes the compromise. This patch reorders the rules according to your comment #12 as desired. To also satisfy my comment #14, I'm making the rules in primaryToolbar.css match those in messengercompose.css, thus to be unambiguous that those are indeed identical rules. I still think this should make it easier for anybody creating new icons to recognize that those actually match without any confusion by slight difference between those implementations. (In reply to Philip Chee from comment #12) > Which was to minimize churn in Hg Blame. Looking at bug 722758's comm-central changeset 2896ec0b8c06, these were all new rules from scratch as well, thus there is in fact no difference to the situation for messagecompose.css here. Anyway, so I'd say either let's stick with what is checked in or change it consistently in both versions.

Comment on attachment 728761 [details] [diff] [review] Follow-up patch (v2, trunk only) > Looking at bug 722758's comm-central changeset 2896ec0b8c06, these were all > new rules from scratch as well, thus there is in fact no difference to the > situation for messagecompose.css here. In this case I'd say WONTFIX for this patch. > Anyway, so I'd say either let's stick with what is checked in or change it > consistently in both versions

Comment on attachment 728761 [details] [diff] [review] Follow-up patch (v2, trunk only) Ok, let's be happy with what we have then until a permanent solution is found. Thus, only the branch approval of attachment 728629 [details] [diff] [review] is left to do.

Looks and works fine in today's SM 2.19a1 nightly (as tested with Linux x86_64 build).

Comment on attachment 728629 [details] [diff] [review] Final patch (v3) [c-c: comment #10] This has missed the last 2.17 beta, thus moving the comm-aurora request for 2.18 to comm-beta after today's merge. [Approval Request Comment] (see comment #9)