Skip to content

Conversation

@kcheney1
Copy link
Contributor

@kcheney1 kcheney1 commented May 27, 2022

896b287

[WK2] Attachment icons do not update after updating filewrapper
https://bugs.webkit.org/show_bug.cgi?id=241026
rdar://86293273

Reviewed by Wenson Hsieh.

In the case of certain iWork file types downloaded from iCloud that require
thumbnails, we don't remove the progress update once the filewrapper is updated
with the complete attachment, which prevents the thumbnail from appearing.

This patch removes the progress attribute once the thumbnail is updated
so we know we can now paint the icon.

No new tests. There's no clear way to test this change. For previous thumbnail bugs
we swizzled the thumbnail generator code to make sure we were generating
thumbnails; however, this bug is a case of the thumbnail being generated but not
displayed. Similarly, we can't compare the attachment size because it is the same for
the attachment with and without the correct thumbnail.

* Source/WebCore/html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::updateThumbnail):

Canonical link: https://commits.webkit.org/251080@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294976 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@kcheney1 kcheney1 self-assigned this May 27, 2022
@kcheney1 kcheney1 added WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). WebKit Nightly Build labels May 27, 2022
@whsieh
Copy link
Member

whsieh commented May 27, 2022

It seemed a tiny bit odd for QL thumbnailing to have the side effect of removing a DOM attribute, but given that this is basically how the rest of the WKAttachment client SPI works — e.g. -setFileWrapper:, etc. — I don't think it's too weird.

@kcheney1 kcheney1 added the merge-queue Applied to send a pull request to merge-queue label May 27, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 896b287 into WebKit:main May 28, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r294976 (251080@main): https://commits.webkit.org/251080@main

Reviewed commits have been landed. Closing PR #1108 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants