Summary: | [chromium] Fix plumbing for differentiating between clipboard/selection pastes. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Cheng <dcheng> | ||||||
Component: | New Bugs | Assignee: | Daniel Cheng <dcheng> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | fishd, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Daniel Cheng
2011-11-10 13:59:32 PST
Created attachment 114570 [details]
Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API. Comment on attachment 114570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114570&action=review > Source/WebKit/chromium/src/PlatformSupport.cpp:153 > +uint64_t PlatformSupport::clipboardGetSequenceNumber(PasteboardPrivate::ClipboardBuffer buffer) if you are going to rename WebClipboard::getSequenceNumber to WebClipboard::sequenceNumber, then it follows that you should also rename clipboardGetSequenceNumber to clipboardSequenceNumber, right? Comment on attachment 114570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114570&action=review >> Source/WebKit/chromium/src/PlatformSupport.cpp:153 >> +uint64_t PlatformSupport::clipboardGetSequenceNumber(PasteboardPrivate::ClipboardBuffer buffer) > > if you are going to rename WebClipboard::getSequenceNumber to WebClipboard::sequenceNumber, then > it follows that you should also rename clipboardGetSequenceNumber to clipboardSequenceNumber, right? Hm. I wasn't sure. I remember hearing somewhere that in WebKit, Class::getX() is usually something that returns a mutable X, so I thought WebClipboard::sequenceNumber would be more appropriate. But it seems like renaming the general convention in PlatformSupport is prefixVerbObject, which clipboardSequenceNumber wouldn't follow. Do you have a preference here? Created attachment 116154 [details]
Patch
(In reply to comment #3) > (From update of attachment 114570 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114570&action=review > > > Source/WebKit/chromium/src/PlatformSupport.cpp:153 > > +uint64_t PlatformSupport::clipboardGetSequenceNumber(PasteboardPrivate::ClipboardBuffer buffer) > > if you are going to rename WebClipboard::getSequenceNumber to WebClipboard::sequenceNumber, then > it follows that you should also rename clipboardGetSequenceNumber to clipboardSequenceNumber, right? I've updated the patch to make the naming consistent in WebKit. Comment on attachment 116154 [details]
Patch
LGTM, I defer to fishd due to the API change.
Comment on attachment 116154 [details]
Patch
Oh, is it possible to write a test for this?
(In reply to comment #8) > (From update of attachment 116154 [details]) > Oh, is it possible to write a test for this? It might be possible, by calling execCommand("copy"), then calling execCommand("paste") in the copy handler, then verifying that reads fail as expected. But the logic for exposing a clipboard sequence number is currently not implemented in the mock clipboard used by DRT, so it would require another 2-sided patch. Comment on attachment 116154 [details]
Patch
WebKit API change LGTM
Committed r101031: <http://trac.webkit.org/changeset/101031> |