Closed Bug 384340 Opened 17 years ago Closed 15 years ago

[Mac Classic] Checkmark instead of alignment-icon in image alignment menulist

Categories

(SeaMonkey :: Composer, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0

People

(Reporter: stefanh, Assigned: stefanh)

References

Details

(Keywords: classic, fixed-seamonkey2.0, regression)

Attachments

(4 files, 3 obsolete files)

Attached image Checkmark instead of alignment-icon (deleted) —
SeaMonkey trunk, MAc OS 10.3.9. STR: 1) Launch Composer and choose "Insert --> Image...". 2) Switch to Appearance tab. 3) Open the "Align Text to Image" menulist. --> The "At the bottom" menuitem has a checkmark (if that is what you have chosen) in front of the label instead of the at the btm alignment icon. See attached screenshot (don't worry about the dot, 10.3.9 messes up the checkmark icon a bit - a core bug). This is a regression from the MOZ_XUL_APP=1 switch. The right icon is there in a xpfe build from 20070525. We seem to get overruled by menu.css.
Keywords: regression
Summary: Checkmark instead of alignment-icon in image alignment menulist → [Mac Classic] Checkmark instead of alignment-icon in image alignment menulist
Actually, I have to check what the correct behaviour is. Thinking of it, I have never seen images/icons in mac menulists (except for the checkmark icons, of course).
I actually see this in thunderbird as well: The menulist in the Message Filters dialog (Tools --> Message Filters) has the same behaviour (no icon in the menulist, though). Samuel, what's your opinion on how it should behave? Oh, and we can't have both icons and checkmarks in the menulist, because that's not possible ;-)
In the bug, as filed, I think you're best off with removing the images from the menu and simply showing the check mark on the selected item. Theoretically, I don't see why both (icon and check mark) can't live in the same menu. This is done is several apps in their main menus; i.e., in the "Bookmarks" menu there are icons but still room for the check mark. It's not pretty, but it's probably doable. But, as I said above, for composer, I'd remove the images. (Is there a way to show the image only when the menu isn't expanded and otherwise show the checkmark? That'd be more ideal.)
Can you reproduce with current SeaMonkey v2.0a1pre ?
Assignee: composer → nobody
QA Contact: composer
Version: unspecified → Trunk
Depends on: 460699
It appears that we have icons in menulist menuitems in several places, so I think the best solution for now is to get rid of the checkmark.
Attached patch Get rid of checkmarks (obsolete) (deleted) — Splinter Review
Optionally, I think just having the checkmark would have been better, but considering we show icons instead of checkmarks in a couple of other places I think this is OK for now. It's at least 200% better than the current look. I removed the icon in the menulist (and the 34px height...), I assume that it's OK for win/nix as well.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #398856 - Flags: superreview?(neil)
Attachment #398856 - Flags: review?(mnyromyr)
Target Milestone: --- → seamonkey2.0
(In reply to comment #6) > I removed the icon in the menulist, I assume that it's OK for win/nix as well. No, it's not. (Removing the height is a good idea though.)
(In reply to comment #7) > (In reply to comment #6) > > I removed the icon in the menulist, I assume that it's OK for win/nix as well. > No, it's not. (Removing the height is a good idea though.) I guess it's ok having icons in menulists, but this icon is 32x26... Note also that the icon is scaled down in mac menuitems since they're dimensioned for 16x16 icons.
Attached image How it looks on windows (deleted) —
Attached image How it looks on mac (deleted) —
Attached patch New version, scale the icon on mac (obsolete) (deleted) — Splinter Review
I'm still not 100% convinced I should scale the icons, but as IanN pointed out - they already get scaled in the menuitems...
Attachment #398856 - Attachment is obsolete: true
Attachment #398886 - Flags: superreview?(neil)
Attachment #398886 - Flags: review?(mnyromyr)
Attachment #398856 - Flags: superreview?(neil)
Attachment #398856 - Flags: review?(mnyromyr)
Attached patch Third attempt, now with some clean-up of (obsolete) (deleted) — Splinter Review
Here's a new version, same as previous one but with some clean-up: 1) Removed groupbox overrides 2) Removed some obsolete style rules
Attachment #398886 - Attachment is obsolete: true
Attachment #398969 - Flags: superreview?(neil)
Attachment #398969 - Flags: review?(mnyromyr)
Attachment #398886 - Flags: superreview?(neil)
Attachment #398886 - Flags: review?(mnyromyr)
Comment on attachment 398969 [details] [diff] [review] Third attempt, now with some clean-up of >-.MinWidth15em { >- min-width: 15em; >-} I'd be tempted to leave this one in for consistency. Maybe for 2.next we can reevaluate these styles and maybe switch them to ch. >+#alignTypeSelect, >+#alignLabel { >+ -moz-margin-start: 5px; I'm not sure why we bother with this. >+/* ::::: menulist icons ::::: */ >+ >+.menulist-icon { >+ max-width: 16px; >+ max-height: 16px; >+} See bug 514854 comment 4, you could override the icon width and height to auto (but only in the .align-menu > .menu-iconic-left > .menu-iconic-icon case).
This one uses the original icon sizes. I'm not really happy with any of these solutions, but the problem here is really that the icons are too big. Scaling makes the icons looks bad and not scaling them makes the menulist look a bit abnormal. :/
Attachment #398969 - Attachment is obsolete: true
Attachment #399112 - Flags: superreview?(neil)
Attachment #399112 - Flags: review?(mnyromyr)
Attachment #398969 - Flags: superreview?(neil)
Attachment #398969 - Flags: review?(mnyromyr)
Actually, I'll remove the bold rule for #MisspelledWord, since it looks like this in the xul: <label class="bold" id="MisspelledWord"/>
Comment on attachment 399112 [details] [diff] [review] Another attempt, use normal icon sizes >-#alignTypeSelect { >- height: 34px; >-} Please move the (following) rule from the mac CSS to here so that we can benefit from it on all platforms, not just Mac. (Compare bug 514854.) r+sr=me with that fixed, since it's then not a Mac-specific fix :-) >+.align-menu > .menu-iconic-left > .menu-iconic-icon { >+ height: auto; >+ width: auto; >+}
Attachment #399112 - Flags: superreview?(neil)
Attachment #399112 - Flags: superreview+
Attachment #399112 - Flags: review?(mnyromyr)
Attachment #399112 - Flags: review+
Comment on attachment 399112 [details] [diff] [review] Another attempt, use normal icon sizes Low risk, css-only. Also, this bug blocks a blocker...
Attachment #399112 - Flags: approval-seamonkey2.0?
(I'll fix comment #15 and comment #16 on check-in)
Attachment #399112 - Flags: approval-seamonkey2.0? → approval-seamonkey2.0+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Adding fixed-seamonkey2.0 keyword so the queries for approved but not fixed bugs don't catch that bug.
Actually, is there a reason for the !important in this line from pinstripe: >list-style-image: url("chrome://global/skin/menu/menu-check-hov.png") !important; Might be worth filing a bug to remove it on trunk.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: