Closed Bug 1273438 Opened 8 years ago Closed 8 years ago

Tools > Options, Composition, General, HTML font selector uses short separator

Categories

(Thunderbird :: Preferences, defect)

Product:
Thunderbird
Component:
Preferences
Platform:
All
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 50.0

People

(Reporter: jorgk-bmo, Assigned: Paenglab)

References

Details

Attachments

(4 files)

bug1273438.patch
4.84 KB, patch
jorgk-bmo
: review+
jorgk-bmo
: feedback+
Details | Diff | Splinter Review
This is how the search engine drop down looks on Win 10.
5.27 KB, image/png
Details
icon spacing with -moz-appearance: menuimage; removed
8.10 KB, image/png
Details
bug1273438.patch v2
5.35 KB, patch
aceman
: review+
Details | Diff | Splinter Review 
Tools > Options, Composition, General, HTML font selector uses short separator.

Discovered when working on bug 1268520.
What platform is this on? I can't see it on Windows XP.
Windows 7, 49.0a1 (2016-05-16). You want to see a screen shot or do you trust me?
Same in 45.1.
No, I just forget on which platform we fixed these separators. So is it a case where the space is reserved for the icons on the menuitems and we need to apply our new class "menulist-without-icons" ?
OS: Unspecified → Windows 7
Hardware: Unspecified → All
(In reply to :aceman from comment #4)
> So is it a
> case where the space is reserved for the icons on the menuitems and we need
> to apply our new class "menulist-without-icons" ?
Yes.
Good :)
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
I'm waiting for your font menulist bug to land first to not rot each other :)
Ready :)
Blocks: 1278489
Attached patch bug1273438.patch — — Splinter Review 
This fixes the issue by adding chrome://messenger/skin/messenger.css to preferences.xul. This makes the menulists behave like the other in TB because it uses the rules I added in bug 1278489.

As an extra the "Search Web Engine" menulistpopup shows now the search engine icons.

I tried to align the icons on the menulist and the popup but on Linux it works only for some themes because the different themes use a different button border width and this makes it impossible to align them for all themes.
Attachment #8771784 - Flags: review?(acelists)
Comment on attachment 8771784 [details] [diff] [review]
bug1273438.patch

Review of attachment 8771784 [details] [diff] [review]:
-----------------------------------------------------------------

I see no icons in the search menupopup, on linux. Is that expected? Only when a search engine is selected, its icon is shown in the menulist parent.

I'd like Jorg so test the separators on Windows 7 as I can't do that.
Attachment #8771784 - Flags: review?(mozilla)
(In reply to :aceman from comment #10)
> Comment on attachment 8771784 [details] [diff] [review]
> bug1273438.patch
> 
> Review of attachment 8771784 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I see no icons in the search menupopup, on linux. Is that expected? Only
> when a search engine is selected, its icon is shown in the menulist parent.

Do you have disabled in your system the show icons in menus?
How do I check? I used Qt/KDE but TB uses GTK3. How do I configure gtk3 themes?
Looks OK to me, however, there could be less space between the icon and the left edge.
Comment on attachment 8771784 [details] [diff] [review]
bug1273438.patch

Review of attachment 8771784 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK, but see previous comment. I'm not sure what the different margin-inline-start/end are about, sometimes 5px, sometimes 3px, etc. Do I need to look at more than what's in my picture? Aceman mentioned separators, which separators? My Win 7 is 1800 km away.
Attachment #8771784 - Flags: feedback+
You filed this bug about "Tools > Options, Composition, General, HTML font selector has short separator". Please check that with the patch. The search engine menu is a bonus :)
Yes, now I recognise my bug ;-) You've hijacked it for those search engine icons so I didn't recognise it at first. Yes, the separator is fine now.
(In reply to :aceman from comment #12)
> How do I check? I used Qt/KDE but TB uses GTK3. How do I configure gtk3
> themes?

I have KDE5 and can set it there I also set the used GTK3 theme.
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #14)
> Comment on attachment 8771784 [details] [diff] [review]
> bug1273438.patch
> 
> Review of attachment 8771784 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks OK, but see previous comment. I'm not sure what the different
> margin-inline-start/end are about, sometimes 5px, sometimes 3px, etc. Do I
> need to look at more than what's in my picture? Aceman mentioned separators,
> which separators? My Win 7 is 1800 km away.

They touch only the menulist button and are to match the spaces the menupopup uses to align the icon/text in the menulist. The spacing for the menu comes from global/menu.css. The icon has also -moz-appearance: menuimage; which makes the spacing around the icon.
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #13)
> Looks OK to me, however, there could be less space between the icon and the
> left edge.
What about this comment? Not too much padding around the icons and especially too much space between left edge and icon? Sorry, I'm really no expert and I'm happy to follow your lead/suggestion. I'll give you the r+ either way, don't worry ;-)
For example on Options > Attachments, Incoming, there is less padding around the icons. Where else do we have menus with icons?
Just noticed, the FF "Default Search Engine" drop-down menu has a lot of padding, too.
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #19)
> (In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #13)
> > Looks OK to me, however, there could be less space between the icon and the
> > left edge.
> What about this comment? Not too much padding around the icons and
> especially too much space between left edge and icon? Sorry, I'm really no
> expert and I'm happy to follow your lead/suggestion. I'll give you the r+
> either way, don't worry ;-)

(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #20)
> For example on Options > Attachments, Incoming, there is less padding around
> the icons. Where else do we have menus with icons?

I attached a screenshot with only removed -moz-appearance: menuimage; (don't check the menulist spacing, this is from a Daily without the patch).
Hmm, less padding. I'd need the patch to try it since on your screenshot the icons are a little fuzzy (is this magnified?) Which one do you like better?
(In reply to Richard Marti (:Paenglab) from comment #17)
> I have KDE5 and can set it there I also set the used GTK3 theme.

So which config panel is it?
(In reply to :aceman from comment #24)
> (In reply to Richard Marti (:Paenglab) from comment #17)
> > I have KDE5 and can set it there I also set the used GTK3 theme.
> 
> So which config panel is it?

System Settings > Application Style > Gnome Application Style (GTK) > Behavior
I do not seem to have such a panel. Can the config program be run from command line?
This patch removes the padding on Windows.
Attachment #8771784 - Attachment is obsolete: true
Attachment #8771784 - Flags: review?(mozilla)
Attachment #8771784 - Flags: review?(acelists)
Attachment #8772966 - Flags: review?(mozilla)
Comment on attachment 8772966 [details] [diff] [review]
bug1273438.patch v2

Review of attachment 8772966 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, I determined I have no icons in all TB menus so it may be a GTK setting. So it is fine I see no icons in the Search engine menu. So I do not see any problems from this patch on Linux.
I let Jorg decide on the Windows part.
Attachment #8772966 - Flags: review+
Can I be terrible? I don't like v2. Icon and text are too close together. And there is still a disproportional space between the left edge of the box and the right edge of the icon.

If let's go with v1. Let me attach picture in a second.
Attachment #8772966 - Flags: review?(mozilla)
Comment on attachment 8771784 [details] [diff] [review]
bug1273438.patch

Let's go with this. Better than v2. Sorry.
Attachment #8771784 - Attachment is obsolete: false
Attachment #8771784 - Flags: review+
Yeah, v2 is too condensed.

https://hg.mozilla.org/comm-central/rev/b61a59f3e1c0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: