Summary: | [macOS] Copying a table from the Numbers app and pasting into iCloud Numbers fails | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||||
Component: | HTML Editing | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aestes, bdakin, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Wenson Hsieh
2018-03-08 15:16:54 PST
Created attachment 335450 [details]
Patch
Created attachment 335458 [details]
Fix macOS 10.11 builds
Comment on attachment 335458 [details] Fix macOS 10.11 builds View in context: https://bugs.webkit.org/attachment.cgi?id=335458&action=review > Source/WebCore/ChangeLog:12 > + r222688) this means we now don't allow web pages to access "text/plain" in the case where the user copies part End the previous sentence and capitalize "this"? > Source/WebCore/ChangeLog:13 > + of a table from the native Numbers app, since Numbers additionally writes a snapshot of the table to the No need for , here. > Source/WebCore/ChangeLog:16 > + In the first place, this restriction on getData/setData was intended to prevent web pages from extracting users' I don't think "In the first place" as much value to the description here. > Source/WebCore/ChangeLog:37 > + Only whitelist "text/html" or "text/uri-list" in the case where there are actual files in the pasteboard. If Only allow*. We should avoid using terms such as whitelist and blacklist going forward. > Source/WebCore/ChangeLog:47 > + - MayContainFilePaths indicates that there are real might be real file paths on the pasteboard. This means Nit: there are real might be real file paths -> there might be file paths. > Source/WebCore/dom/DataTransfer.cpp:207 > + return m_pasteboard->containsFiles() == Pasteboard::ContainsFilesResult::MayContainFilePaths; Now that this function returns an enum, it's strange to call this function "containsFile". How about m_pasteboard->fileContentState()? > Source/WebCore/platform/Pasteboard.h:224 > + enum class ContainsFilesResult { NoImageDataOrFilePaths, OnlyImageData, MayContainFilePaths }; > + virtual WEBCORE_EXPORT ContainsFilesResult containsFiles(); How about FileContentState { NoFileOrImageData, InMemoryImage, MayContainFilePaths } and fileContentState(). I don't think "OnlyImageData" quite communicate as to why that's relevant to the presence of a file. Comment on attachment 335458 [details] Fix macOS 10.11 builds View in context: https://bugs.webkit.org/attachment.cgi?id=335458&action=review >> Source/WebCore/ChangeLog:12 >> + r222688) this means we now don't allow web pages to access "text/plain" in the case where the user copies part > > End the previous sentence and capitalize "this"? Fixed! >> Source/WebCore/ChangeLog:13 >> + of a table from the native Numbers app, since Numbers additionally writes a snapshot of the table to the > > No need for , here. Removed the "," >> Source/WebCore/ChangeLog:16 >> + In the first place, this restriction on getData/setData was intended to prevent web pages from extracting users' > > I don't think "In the first place" as much value to the description here. Fair enough — removed! >> Source/WebCore/ChangeLog:37 >> + Only whitelist "text/html" or "text/uri-list" in the case where there are actual files in the pasteboard. If > > Only allow*. We should avoid using terms such as whitelist and blacklist going forward. Ah, right! Fixed. >> Source/WebCore/ChangeLog:47 >> + - MayContainFilePaths indicates that there are real might be real file paths on the pasteboard. This means > > Nit: there are real might be real file paths -> there might be file paths. Whoops, fixed. >> Source/WebCore/dom/DataTransfer.cpp:207 >> + return m_pasteboard->containsFiles() == Pasteboard::ContainsFilesResult::MayContainFilePaths; > > Now that this function returns an enum, it's strange to call this function "containsFile". > How about m_pasteboard->fileContentState()? Good point — changed Pasteboard::containsFile() to Pasteboard::fileContentState() >> Source/WebCore/platform/Pasteboard.h:224 >> + virtual WEBCORE_EXPORT ContainsFilesResult containsFiles(); > > How about FileContentState { NoFileOrImageData, InMemoryImage, MayContainFilePaths } and fileContentState(). > I don't think "OnlyImageData" quite communicate as to why that's relevant to the presence of a file. Yeah, this sounds reasonable. Renamed enum type and values to FileContentState { NoFileOrImageData, InMemoryImage, MayContainFilePaths } (And also renamed the method to fileContentState). Created attachment 335506 [details]
One more EWS run
Created attachment 335507 [details]
One more EWS run
Comment on attachment 335507 [details] One more EWS run Clearing flags on attachment: 335507 Committed r229503: <https://trac.webkit.org/changeset/229503> |