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

RESOLVED FIXED in Thunderbird 50.0

Status

Thunderbird
Preferences
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jorg K (GMT+2) (bustage-fix and urgent reviews only), Assigned: Paenglab)

Tracking

Trunk
Thunderbird 50.0
All
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

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

Discovered when working on bug 1268520.

Comment 1

2 years ago
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.

Comment 4

2 years ago
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.

Comment 6

2 years ago
Good :)
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
(Assignee)

Comment 7

2 years ago
I'm waiting for your font menulist bug to land first to not rot each other :)

Comment 8

2 years ago
Ready :)
Blocks: 1278489
(Assignee)

Comment 9

2 years ago
Created attachment 8771784 [details] [diff] [review]
bug1273438.patch

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 10

2 years ago
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)
(Assignee)

Comment 11

2 years ago
(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?

Comment 12

2 years ago
How do I check? I used Qt/KDE but TB uses GTK3. How do I configure gtk3 themes?
Created attachment 8772576 [details]
This is how the search engine drop down looks on Win 10.

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+

Comment 15

2 years ago
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.
(Assignee)

Comment 17

2 years ago
(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.
(Assignee)

Comment 18

2 years ago
(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.
(Assignee)

Comment 22

2 years ago
Created attachment 8772794 [details]
icon spacing with -moz-appearance: menuimage; removed

(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?

Comment 24

2 years ago
(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?
(Assignee)

Comment 25

2 years ago
(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

Comment 26

2 years ago
I do not seem to have such a panel. Can the config program be run from command line?
(Assignee)

Comment 27

2 years ago
Created attachment 8772966 [details] [diff] [review]
bug1273438.patch v2

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 28

2 years ago
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+
(Assignee)

Comment 31

2 years ago
Yeah, v2 is too condensed.

https://hg.mozilla.org/comm-central/rev/b61a59f3e1c0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 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.