Summary: | [chromium] Add an interface for platform copy/paste drag/drop data objects | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Cheng <dcheng> | ||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, tony | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 44727, 44917 | ||||||||
Attachments: |
|
Description
Daniel Cheng
2010-08-30 17:44:04 PDT
Created attachment 65985 [details]
Patch
Comment on attachment 65985 [details] Patch > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > + [chromium] Add an interface for platform copy/paste drag/drop data objects > + https://bugs.webkit.org/show_bug.cgi?id=44914 Nit: Please add a sentence explaining the general goal (ie., something about so we can eventually support arbitrary data types). > diff --git a/WebCore/platform/chromium/ChromiumDataObjectNew.h b/WebCore/platform/chromium/ChromiumDataObjectNew.h > + virtual String getData(const String& type, bool& succeeded) const = 0; > + virtual bool setData(const String& type, const String& data) = 0; It seems like these methods should use the same style. Two possibilities: both return a bool (on success) or just remove the bool param complete from getdata (do we need to differentiate between empty and fail?). The other getters below don't seem to use a bool. Created attachment 65989 [details] Patch (In reply to comment #2) > (From update of attachment 65985 [details]) > > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > > + [chromium] Add an interface for platform copy/paste drag/drop data objects > > + https://bugs.webkit.org/show_bug.cgi?id=44914 > > Nit: Please add a sentence explaining the general goal (ie., something about so we can eventually support arbitrary data types). Done. Updating the other patch as well. > > > diff --git a/WebCore/platform/chromium/ChromiumDataObjectNew.h b/WebCore/platform/chromium/ChromiumDataObjectNew.h > > + virtual String getData(const String& type, bool& succeeded) const = 0; > > + virtual bool setData(const String& type, const String& data) = 0; > > It seems like these methods should use the same style. Two possibilities: both return a bool (on success) or just remove the bool param complete from getdata (do we need to differentiate between empty and fail?). The other getters below don't seem to use a bool. This is inherited from Clipboard's getData() / setData() signature. I agree it's not ideal, but I don't feel comfortable changing Clipboard. (In reply to comment #3) > This is inherited from Clipboard's getData() / setData() signature. I agree it's not ideal, but I don't feel comfortable changing Clipboard. Please file a bug for this. Bonus points for actually fixing. Refactoring is important work! Comment on attachment 65989 [details] Patch Clearing flags on attachment: 65989 Committed r66483: <http://trac.webkit.org/changeset/66483> All reviewed patches have been landed. Closing bug. |