Bug 242269
| Summary: | Rename 'imageAndSize' and 'image' as 'iconAndSize' and 'icon' | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> |
| Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | bfulgham, karlcow, webkit-bug-importer |
| Priority: | P2 | Keywords: | GoodFirstBug, InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Per Arne Vollan
Address review feedback in https://github.com/WebKit/WebKit/pull/1928:
This function WebPageProxy::iconForAttachment is confusing.
1) 'imageAndSize' and 'image' should be renamed to 'iconAndSize' and 'icon' since the type is IconAndSize and the data member is .icon.
2) 'size' should be part of the return value. It's super subtle that 'size' is an out parameter -- but only on one platform!
3) Look into creating an API test for this
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/96315732>
Karl Dubost
https://github.com/WebKit/WebKit/blob/6b7f7447287dd93b8742b5e24ab12047cf6edf48/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm#L356-L366
current code
RefPtr<WebCore::ShareableBitmap> WebPageProxy::iconForAttachment(const String& fileName, const String& contentType, const String& title, FloatSize& size)
{
#if PLATFORM(IOS_FAMILY)
auto imageAndSize = RenderThemeIOS::iconForAttachment(fileName, contentType, title);
auto image = imageAndSize.icon;
size = imageAndSize.size;
#else
auto image = RenderThemeMac::iconForAttachment(fileName, contentType, title);
#endif
return convertPlatformImageToBitmap(image.get(), iconSize);
}
Adding GoodFirstBug
Ian Gower
Pull request: https://github.com/WebKit/WebKit/pull/46092
Brent Fulgham
Follow-on work is being done in Bug 293778.
Ian Gower
Pull request: https://github.com/WebKit/WebKit/pull/46147
EWS
Committed 295660@main (243f58ea8132): <https://commits.webkit.org/295660@main>
Reviewed commits have been landed. Closing PR #46147 and removing active labels.