Close Tab Icon mispositioned

RESOLVED FIXED in Instantbird 43

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: freaktechnik, Assigned: Paenglab)

Tracking

trunk
Instantbird 43
Unspecified
Windows 10

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
The close tab icon is misaligned both in its normal and hover states since the latest nightly on Windows 10. See the attached screenshot.
(Assignee)

Comment 1

4 years ago
This is a fallout of bug 1173729. If the old icon is still wanted then with referring to chrome://global/skin/icons/close-XPVista7.png would show the 16px icon. Or with changing to the close-icon class the new icon would be used on win 8 and 10 (and the old on the older platforms). But the new icon on win 8 and 10 is now 20px and it doesn't look good with this tabs.
(Reporter)

Comment 2

4 years ago
The same thing happens with the x icon (I don't know its actual functionality) for groups in the contacts list.
(Assignee)

Comment 3

4 years ago
Posted patch 1192999.patch (obsolete) — Splinter Review
This patch uses close-XPVista7.png for all windows platforms. It removes also a change I made for TB which is no more needed now with this patch.
Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8645983 - Flags: review?(aleth)

Comment 4

4 years ago
Comment on attachment 8645983 [details] [diff] [review]
1192999.patch

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

I can't test this, so bouncing the review over to clokep.
Attachment #8645983 - Flags: review?(aleth) → review?(clokep)
This looks OK for the tabbar on Windows. The buddy list is still wrong, however. I'll try to find that CSS and change it.

I haven't tested the Thunderbird changes yet.
(Assignee)

Comment 6

4 years ago
Hmm, on TB the closeConversationButton looks good with this patch. I don't use IB, does the buddy list not use closeConversationButton?
(Reporter)

Comment 7

4 years ago
The close icons in the buddy list is .hideGroupButton: https://dxr.mozilla.org/comm-central/source/im/themes/blist.css#197
(Assignee)

Comment 8

4 years ago
Posted patch 1192999.patchSplinter Review
blist.css added to the patch.
Attachment #8645983 - Attachment is obsolete: true
Attachment #8645983 - Flags: review?(clokep)
Attachment #8646279 - Flags: review?(clokep)
Comment on attachment 8646279 [details] [diff] [review]
1192999.patch

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

Looks good. Thanks Richard!

At least the im/ part of this patch needs to be uplifted to 42.
Attachment #8646279 - Flags: review?(clokep) → review+
(Assignee)

Comment 10

4 years ago
Also the TB part can be uplifted.
Keywords: checkin-needed
(Assignee)

Comment 11

4 years ago
Does IM have no approval flags?
https://hg.mozilla.org/comm-central/rev/d4712ef1438f

We do not currently have approval flags. I'm not sure what our plan is for that.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(florian)
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 43
(In reply to Patrick Cloke [:clokep] from comment #12)
> https://hg.mozilla.org/comm-central/rev/d4712ef1438f
> 
> We do not currently have approval flags. I'm not sure what our plan is for
> that.

We'll need some, but it doesn't seem urgent currently.
Flags: needinfo?(florian)
Since Instantbird is planning to release on 42, this needs to be backported. This was tested for a few days in nightlies and seems to work fine. The change in icon is a low-risk to backport.

I'm applying this to comm-aurora:

https://hg.mozilla.org/releases/comm-aurora/rev/2679472bfea5
You need to log in before you can comment on or make changes to this bug.