Closed Bug 1730334 Opened 11 months ago Closed 10 months ago

Attachment icons stretched on high-DPI monitor

Categories

(Thunderbird :: Mail Window Front End, defect)

Thunderbird 91
defect

Tracking

(thunderbird_esr91+ fixed, thunderbird94 wontfix)

RESOLVED FIXED
95 Branch
Tracking Status
thunderbird_esr91 + fixed
thunderbird94 --- wontfix

People

(Reporter: fanormand, Assigned: henry)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(17 files)

30.24 KB, image/jpeg
Details
79.84 KB, image/jpeg
Details
36.00 KB, image/jpeg
Details
81.41 KB, image/jpeg
Details
9.83 KB, image/jpeg
Details
4.78 KB, text/plain
Details
4.78 KB, application/octet-stream
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
816 bytes, patch
aleca
: review+
Details | Diff | Splinter Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Open a message containing attachments.

Actual results:

Icons are stretched in attachments pane (see screenshot).

Tested on Thunderbird 91.0.3 for macOS (high-DPI monitor).

Flags: needinfo?(henry)
Blocks: tb91found

I cannot reproduce this, but I also do not have a high dpi screen. It is probably the xul layout being bad scaling html images, which I have seen before. This one is a little bit surprising though because the moz-icon: request specifies a size of 16 https://searchfox.org/comm-central/rev/2392baf95c8a9513a39f70e123779d12a0872376/mail/base/content/widgets/mailWidgets.js#1732 , but I guess the actual image may be larger.

If it is what I suspect, then just changing the parent to html (or just wrapping it in a div) works to fix it. A cleaner solution would be to change the whole attachmentcell-content structure to html (and its semantics can be improved along the way). I could fix this, but I would just need someone to test it with a high dpi monitor to be sure its actually working.

Flags: needinfo?(henry)
Regressed by: 1683865
Component: Untriaged → Mail Window Front End
Summary: Attachment icons stretched → Attachment icons stretched on high-DPI monitor

I can confirm the bug. Tested with Thunderbird 93.0b4 (64-bit) on a UHD screen (3840x2160) under Windows 10 Pro.

Status: UNCONFIRMED → NEW
Ever confirmed: true

I don't see this with TB 91 and Daily on Windows 10 and UHD screen. What scaling do you use and does it still happen in Troubleshoot mode (see Help menu)?

It actually has to do with scaling in Windows. The problem does not occur without scaling, with the scaling of 200% set for me it also occurs in Thunderbird 91.1.1 (64-bit, troubleshoot mode); please see attached screenshots.

I tried 100%, 125%, 150%, 175%, 200% and 225% and never saw the stretching. I started TB on this screen and also on another one and moved to the scaled screen, no problem visible.

My last proposal: try to disable or enable HW-acceleration in Prefs/General at the bottom.

@Richard: I can reproduce in Windows 10 too (125 % scaling, after Thunderbird restart). HW-acceleration enabled or not.

@Richard: Switching hardware acceleration on or off has no effect.

I can try and fix this by converting the structure to html. That has fixed this sort of issue in other situations, and people experiencing the problem can test if it works.

Out of curiosity, for people experiencing the problem, can you test the dimensions of the img element in browser toolbox. Specifically, with an attachment icon visible, open up the console and get the img element with

img = document.querySelector(".attachmentcell-icon")

and then test if any of its dimensional properties - .height, .width, .naturalHeight, .naturalWidth - differ from 16.

Assignee: nobody → henry
Status: NEW → ASSIGNED
@Henry: icon width is proportionnal to scale (100% -> 16px, 125% -> 20px, 150% -> 24px...)

```

```

@Henry: icon width is proportional to scale (100% -> 16px, 125% -> 20px, 150% -> 24px...).

This is needed for the windows platform where a vertical margin is set.

Depends on D127285

Using a CSS rule obscures the logic, when we can just also handle descendant targets.

Depends on D127288

Attachment #9243967 - Attachment description: Bug 1730334 - Stop using pointer-events: none on attachment items. r=aleca → Bug 1730334 - Remove excess attachment list click handler. r=aleca

Also fixed a typo.

Depends on D127976

If the attachment's full name would overflow and needs to be cropped, we still want to see the extension part at the end.

Depends on D127288

Target Milestone: --- → 95 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/a9015ae2a270
Drop the view and orient attributes for attachment-lists. r=aleca
https://hg.mozilla.org/comm-central/rev/37d5cbe97cc7
Include attachmentItem margin in attachmentBucket min-height calculation. r=aleca
https://hg.mozilla.org/comm-central/rev/10911cc92082
Move attachment item name and size logic into MozAttachmentList. r=aleca
https://hg.mozilla.org/comm-central/rev/2eeffb03e098
Change attachmentItem internal structure to html. r=aleca
https://hg.mozilla.org/comm-central/rev/4cd3e226c6d8
Remove excess attachment list click handler. r=aleca
https://hg.mozilla.org/comm-central/rev/6233d6a6a750
Tidy the bigFileObserver event handler. r=aleca
https://hg.mozilla.org/comm-central/rev/d30258ef0b20
Tidy openpgp attachment listeners. r=aleca
https://hg.mozilla.org/comm-central/rev/566075f60ea0
Tidy the attachment list hideDropMarker method. r=aleca
https://hg.mozilla.org/comm-central/rev/1a3eaca4918a
Always show an attachment's extension in attachment lists. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED

I tested copying the trunk commits on top of esr91, but there are several merge conflicts, and they don't all have an easy resolutions. It's probably safer to go with this one-line patch for esr91 instead. It's a bit of a hack because a html:img shouldn't need a CSS min-width when a width is already set, but the xul:hbox doesn't respect this.

Attachment #9247514 - Flags: review?(alessandro)
Comment on attachment 9247514 [details] [diff] [review]
1730334-esr91.patch

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

This fixes the problem on 91, thanks.
Attachment #9247514 - Flags: review?(alessandro) → review+

Comment on attachment 9247514 [details] [diff] [review]
1730334-esr91.patch

[Approval Request Comment]
Regression caused by (bug #): 1683865
User impact if declined: Attachment icons are stretched if they are scaled
Testing completed (on c-c, etc.): No
Risk to taking this patch (and alternatives if risky): Low risk. This is just a single additional CSS rule. This patch diverges from the fix on trunk, but is much simpler (see comment 27).
Note: I'm not putting in a beta request for either this patch or the ones for trunk since they are destined for the next beta anyway.

Attachment #9247514 - Flags: approval-comm-esr91?

Comment on attachment 9247514 [details] [diff] [review]
1730334-esr91.patch

[Triage Comment]
Approved for esr91

Attachment #9247514 - Flags: approval-comm-esr91? → approval-comm-esr91+
You need to log in before you can comment on or make changes to this bug.