| Summary: | Undownloaded iCloud Photos are inserted as broken images when attachment element is enabled | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Component: | HTML Editing | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | akeerthi, darin, ews-watchlist, hi, megan_gardner, mifenton, thorton, webkit-bug-importer, wenson_hsieh | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=234851 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Wenson Hsieh
2022-01-02 16:49:46 PST
Created attachment 448192 [details]
For EWS
Created attachment 448200 [details]
Fix iOS build
Comment on attachment 448200 [details] Fix iOS build View in context: https://bugs.webkit.org/attachment.cgi?id=448200&action=review > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:729 > + bool fileExists = FileSystem::fileExists(path); Instead of making a separate file system call, can we save the result from fileTypeFollowingSymlinks? Always nice to reduce the number of separate file system accesses. > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:736 > + attachment->setAttributeWithoutSynchronization(HTMLNames::progressAttr, "0"_s); I think there’s a way to get the string “0” without allocating a new string. Part of how JavaScript optimizes numeric strings. Not sure it’s easily available, but maybe AtomString::number(0) would work. Comment on attachment 448200 [details] Fix iOS build View in context: https://bugs.webkit.org/attachment.cgi?id=448200&action=review Thanks for the review! >> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:729 >> + bool fileExists = FileSystem::fileExists(path); > > Instead of making a separate file system call, can we save the result from fileTypeFollowingSymlinks? Always nice to reduce the number of separate file system accesses. Seems to work! Looks like the return value is `nullopt` in the case where the file doesn't exist. >> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:736 >> + attachment->setAttributeWithoutSynchronization(HTMLNames::progressAttr, "0"_s); > > I think there’s a way to get the string “0” without allocating a new string. Part of how JavaScript optimizes numeric strings. Not sure it’s easily available, but maybe AtomString::number(0) would work. Yes, `AtomString::number(0)` works — it looks like we already use it right below, too. Created attachment 448246 [details]
For landing
Committed r287547 (245682@main): <https://commits.webkit.org/245682@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448246 [details]. |