Bug 191844 - Insert <attachment> elements under editable images to make their backing data accessible
Summary: Insert <attachment> elements under editable images to make their backing data...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-19 15:13 PST by Tim Horton
Modified: 2018-11-26 17:37 PST (History)
11 users (show)

See Also:


Attachments
WIP (46.13 KB, patch)
2018-11-19 15:20 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (67.74 KB, patch)
2018-11-22 01:01 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (69.10 KB, patch)
2018-11-24 19:04 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.54 MB, application/zip)
2018-11-24 21:17 PST, EWS Watchlist
no flags Details
Patch (69.08 KB, patch)
2018-11-24 23:38 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (69.00 KB, patch)
2018-11-26 00:43 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (68.19 KB, patch)
2018-11-26 16:54 PST, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2018-11-19 15:13:36 PST
Insert <attachment> elements under editable images to make their backing data accessible
Comment 1 Tim Horton 2018-11-19 15:15:28 PST
<rdar://problem/30900149>
Comment 2 Tim Horton 2018-11-19 15:20:00 PST
Created attachment 355297 [details]
WIP
Comment 3 Tim Horton 2018-11-19 15:20:23 PST
Gonna try to write a test.
Comment 4 EWS Watchlist 2018-11-19 15:23:00 PST Comment hidden (obsolete)
Comment 5 Tim Horton 2018-11-22 01:01:16 PST
Created attachment 355463 [details]
Patch
Comment 6 EWS Watchlist 2018-11-22 01:04:56 PST Comment hidden (obsolete)
Comment 7 Wenson Hsieh 2018-11-22 11:59:30 PST
Comment on attachment 355463 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=355463&action=review

The attachment-related changes look good to me!

> Source/WebCore/ChangeLog:38
> +        Store the aformentioned bits of information when cloned so that we can

Nit - aforementioned?

> Source/WebKit/UIProcess/API/APIAttachment.h:97
> +    mutable RetainPtr<NSFileWrapper> m_fileWrapper;

I think m_fileWrapper and m_fileWrapperGenerator should both be under PLATFORM(COCOA), or both be present everywhere.

> Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm:194
> +    m_fileWrapperGenerator = WTFMove(fileWrapperGenerator);

Should we also invalidateGeneratedFileWrapper here?

> Source/WebKit/UIProcess/ios/EditableImageController.mm:51
> +    if (auto webPageProxy = m_webPageProxy.get())

Nit - auto*?
Comment 8 Tim Horton 2018-11-24 19:04:10 PST
Created attachment 355572 [details]
Patch
Comment 9 EWS Watchlist 2018-11-24 19:06:19 PST Comment hidden (obsolete)
Comment 10 EWS Watchlist 2018-11-24 21:17:26 PST Comment hidden (obsolete)
Comment 11 EWS Watchlist 2018-11-24 21:17:28 PST Comment hidden (obsolete)
Comment 12 Tim Horton 2018-11-24 23:25:11 PST
Comment on attachment 355579 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

Crashing test definitely does not appear to be relevant
Comment 13 Tim Horton 2018-11-24 23:38:19 PST
Created attachment 355584 [details]
Patch
Comment 14 EWS Watchlist 2018-11-24 23:41:27 PST
Attachment 355584 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:493:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:494:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:495:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 3 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Tim Horton 2018-11-26 00:43:29 PST
Created attachment 355626 [details]
Patch
Comment 16 EWS Watchlist 2018-11-26 00:46:16 PST
Attachment 355626 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:493:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:494:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:495:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 3 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Simon Fraser (smfr) 2018-11-26 13:49:32 PST
Comment on attachment 355626 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=355626&action=review

> Source/WebCore/html/HTMLImageElement.cpp:337
> +        updateEditableImage(true);

Don't much like the boolean argument.
Comment 18 Tim Horton 2018-11-26 16:54:34 PST
Created attachment 355695 [details]
Patch
Comment 19 EWS Watchlist 2018-11-26 16:56:17 PST
Attachment 355695 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:493:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:494:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/ChromeClient.h:495:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 3 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 WebKit Commit Bot 2018-11-26 17:37:31 PST
Comment on attachment 355695 [details]
Patch

Clearing flags on attachment: 355695

Committed r238538: <https://trac.webkit.org/changeset/238538>
Comment 21 WebKit Commit Bot 2018-11-26 17:37:33 PST
All reviewed patches have been landed.  Closing bug.