59264 2011-04-22 17:55:25 -0700 [chromium] Implement EditorClient::canCopyCut and EditorClient::canPaste 2011-04-28 15:14:01 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 dcheng dcheng abarth commit-queue eric fishd jam rniwa webkit.review.bot oldest_to_newest 391281 0 dcheng 2011-04-22 17:55:25 -0700 This allows the browser to decide if copy/cut/paste operations should be allowed on a per-origin basis, e.g. for extensions. 391294 1 90822 dcheng 2011-04-22 18:10:51 -0700 Created attachment 90822 Patch 391296 2 dcheng 2011-04-22 18:11:05 -0700 For reference, the relevant Chrome patch can be found at http://codereview.chromium.org/6480106/ 392114 3 90822 fishd 2011-04-25 16:50:05 -0700 Comment on attachment 90822 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90822&action=review > Source/WebKit/chromium/public/WebViewClient.h:318 > + virtual bool canTriggerClipboardRead(const WebURL& url) { return false; } nit: webkit style is to drop the parameter name when the name is just a clone of the type name. nit: these functions are listed in the "Zoom" section, but they don't seem to have anything to do with zooming. it looks like registerProtocolHandler was improperly placed here too! 392128 4 91024 dcheng 2011-04-25 16:56:40 -0700 Created attachment 91024 Patch 392129 5 dcheng 2011-04-25 16:57:14 -0700 (In reply to comment #3) > (From update of attachment 90822 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90822&action=review > > > Source/WebKit/chromium/public/WebViewClient.h:318 > > + virtual bool canTriggerClipboardRead(const WebURL& url) { return false; } > > nit: webkit style is to drop the parameter name when the name is just a clone of the type name. > Done. > nit: these functions are listed in the "Zoom" section, but they don't seem to have anything > to do with zooming. it looks like registerProtocolHandler was improperly placed here too! I've picked (what I hope is) a more appropriate location. 392798 6 91024 eric 2011-04-26 15:19:34 -0700 Comment on attachment 91024 Patch Since Darin sounded generally OK with this, this seems OK to me too. 392902 7 91024 commit-queue 2011-04-26 15:59:17 -0700 Comment on attachment 91024 Patch Clearing flags on attachment: 91024 Committed r84967: <http://trac.webkit.org/changeset/84967> 392903 8 commit-queue 2011-04-26 15:59:22 -0700 All reviewed patches have been landed. Closing bug. 393070 9 webkit.review.bot 2011-04-26 16:51:39 -0700 http://trac.webkit.org/changeset/84967 might have broken SnowLeopard Intel Release (WebKit2 Tests) 393900 10 jam 2011-04-27 15:44:13 -0700 Darin: the chrome side of this change, http://codereview.chromium.org/6480106/, is related to extensions. Since we'll want to have the IPCs be in chrome, not content layer, adding these APIs to WebViewClient means that we'll have to implement these virtual functions on RenderView, only to call out to the embedder using ContentRendererClient, and then implement this for real in ChromeContentRendererClient. For autofill/spellcheck/devtools, we've created specific interfaces that the webkit layer calls, and implement those in the chrome layer directly. Seems we should do the same here to avoid all the indirection. sounds good? if so, what name do you recommend? WebClipboardPermissionClient? (I suck with naming :) ). thanks, and apologies to Daniel for all the run-around. 393961 11 fishd 2011-04-27 16:37:25 -0700 (In reply to comment #10) > Darin: the chrome side of this change, http://codereview.chromium.org/6480106/, is related to extensions. Since we'll want to have the IPCs be in chrome, not content layer, adding these APIs to WebViewClient means that we'll have to implement these virtual functions on RenderView, only to call out to the embedder using ContentRendererClient, and then implement this for real in ChromeContentRendererClient. > > For autofill/spellcheck/devtools, we've created specific interfaces that the webkit layer calls, and implement those in the chrome layer directly. Seems we should do the same here to avoid all the indirection. sounds good? if so, what name do you recommend? WebClipboardPermissionClient? (I suck with naming :) ). thanks, and apologies to Daniel for all the run-around. Ah, good point. A separate WebClipboard{Permission,Policy}Client interface SGTM. We could maybe just call it WebClipboardClient. class WebClipboardClient { public: virtual bool canReadFromClipboard(WebFrame*) = 0; virtual bool canWriteToClipboard(WebFrame*) = 0; }; We could just have the methods be named canRead and canWrite, but that wouldn't scale well if we ever had a number of other read/write policy related client interfaces, and someone wanted to implement more than one of them. Note: I think it would be better to pass the WebFrame instead of the URL, so that the caller can have access to more context if needed. 394040 12 jam 2011-04-27 18:05:37 -0700 Darin: it looks like it would be nice to move the following as well out of RenderView. How about a more generic Web{Permission,Policy}Client? If this sounds good, I can take care of doing this, since it would help with some content refactoring I'd like to do in the browser process. virtual bool allowImages(WebKit::WebFrame* frame, bool enabled_per_settings); virtual bool allowPlugins(WebKit::WebFrame* frame, bool enabled_per_settings); virtual bool allowScript(WebKit::WebFrame* frame, bool enabled_per_settings); virtual bool allowDatabase(WebKit::WebFrame* frame, const WebKit::WebString& name, const WebKit::WebString& display_name, unsigned long estimated_size); virtual void didNotAllowScript(WebKit::WebFrame* frame); virtual void didNotAllowPlugins(WebKit::WebFrame* frame); virtual bool allowScriptExtension(WebKit::WebFrame*, const WebKit::WebString& extension_name, int extensionGroup); 394224 13 jam 2011-04-27 22:43:59 -0700 (In reply to comment #12) > Darin: it looks like it would be nice to move the following as well out of RenderView. How about a more generic Web{Permission,Policy}Client? If this sounds good, I can take care of doing this, since it would help with some content refactoring I'd like to do in the browser process. just to be clear, this isn't just for cleaning up the browser process (although it will). most of the functions above go to ContentRendererClient, so it'd be good to get them to go directly to the class that deals with them. I had added them to ContentRendererClient so that we can get rid of the remaining dependencies and turn on DEPS checking 394799 14 jam 2011-04-28 15:14:01 -0700 for anyone reading, this was committed in r85241. 90822 2011-04-22 18:10:51 -0700 2011-04-25 16:56:37 -0700 Patch bug-59264-20110422181050.patch text/plain 2639 dcheng U3VidmVyc2lvbiBSZXZpc2lvbjogODQ2ODUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKaW5kZXgg NzJkNGIwNTA2NjdjZjMzMmNmY2ZkZGFmNjkzMDdjNmY4ZTg2NTY2YS4uYTkyYTgwODQzMmFlOGMy NGZmMDYyZmRmYzJlMjJjMmU1MTQxNzdkNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKQEAg LTEsMyArMSwxNyBAQAorMjAxMS0wNC0yMiAgRGFuaWVsIENoZW5nICA8ZGNoZW5nQGNocm9taXVt Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBb Y2hyb21pdW1dIEltcGxlbWVudCBFZGl0b3JDbGllbnQ6OmNhbkNvcHlDdXQgYW5kIEVkaXRvckNs aWVudDo6Y2FuUGFzdGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTU5MjY0CisKKyAgICAgICAgKiBwdWJsaWMvV2ViVmlld0NsaWVudC5oOgorICAgICAg ICAoV2ViS2l0OjpXZWJWaWV3Q2xpZW50OjpjYW5UcmlnZ2VyQ2xpcGJvYXJkUmVhZCk6CisgICAg ICAgIChXZWJLaXQ6OldlYlZpZXdDbGllbnQ6OmNhblRyaWdnZXJDbGlwYm9hcmRXcml0ZSk6Cisg ICAgICAgICogc3JjL0VkaXRvckNsaWVudEltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpFZGl0 b3JDbGllbnRJbXBsOjpjYW5Db3B5Q3V0KToKKyAgICAgICAgKFdlYktpdDo6RWRpdG9yQ2xpZW50 SW1wbDo6Y2FuUGFzdGUpOgorCiAyMDExLTA0LTIyICBNaWtoYWlsIE5hZ2Fub3YgIDxtbmFnYW5v dkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgUGF2ZWwgRmVsZG1hbi4KZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZpZXdDbGllbnQuaCBi L1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZpZXdDbGllbnQuaAppbmRleCA1MTc0 MzUxMzYyNmMwMzk2MjZlNmQ4ZTY2MGRkODY5Y2VlNzQxNzhhLi42NWZiZmIwMWEzNTAwYWJjMDJj MTkxZTE1YTA2NjAwZTdlOTc5MWIwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVt L3B1YmxpYy9XZWJWaWV3Q2xpZW50LmgKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9wdWJs aWMvV2ViVmlld0NsaWVudC5oCkBAIC0zMTUsNiArMzE1LDkgQEAgcHVibGljOgogICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjb25zdCBXZWJTdHJpbmcmIHVybCwKICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgY29uc3QgV2ViU3RyaW5nJiB0 aXRsZSkgeyB9CiAKKyAgICB2aXJ0dWFsIGJvb2wgY2FuVHJpZ2dlckNsaXBib2FyZFJlYWQoY29u c3QgV2ViVVJMJiB1cmwpIHsgcmV0dXJuIGZhbHNlOyB9CisgICAgdmlydHVhbCBib29sIGNhblRy aWdnZXJDbGlwYm9hcmRXcml0ZShjb25zdCBXZWJVUkwmIHVybCkgeyByZXR1cm4gZmFsc2U7IH0K KwogcHJvdGVjdGVkOgogICAgIH5XZWJWaWV3Q2xpZW50KCkgeyB9CiB9OwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvRWRpdG9yQ2xpZW50SW1wbC5jcHAgYi9Tb3VyY2Uv V2ViS2l0L2Nocm9taXVtL3NyYy9FZGl0b3JDbGllbnRJbXBsLmNwcAppbmRleCA3Y2M4ZjNjYzlk NjBhNjEyZDcwYTFkNjUxMWM4NmExZTVkMzkwMmIyLi5hNmJjM2EzOWM5NmI1YmE0NTc1YjM2NjAx YmFhNGM5ODIzMjhiZTI0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9F ZGl0b3JDbGllbnRJbXBsLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9FZGl0 b3JDbGllbnRJbXBsLmNwcApAQCAtMzIwLDE0ICszMjAsMTQgQEAgdm9pZCBFZGl0b3JDbGllbnRJ bXBsOjpjbGVhclVuZG9SZWRvT3BlcmF0aW9ucygpCiAgICAgbV9yZWRvU3RhY2suY2xlYXIoKTsK IH0KIAotYm9vbCBFZGl0b3JDbGllbnRJbXBsOjpjYW5Db3B5Q3V0KEZyYW1lKiwgYm9vbCBkZWZh dWx0VmFsdWUpIGNvbnN0Citib29sIEVkaXRvckNsaWVudEltcGw6OmNhbkNvcHlDdXQoRnJhbWUq IGZyYW1lLCBib29sIGRlZmF1bHRWYWx1ZSkgY29uc3QKIHsKLSAgICByZXR1cm4gZGVmYXVsdFZh bHVlOworICAgIHJldHVybiAobV93ZWJWaWV3LT5jbGllbnQoKSAmJiBtX3dlYlZpZXctPmNsaWVu dCgpLT5jYW5UcmlnZ2VyQ2xpcGJvYXJkUmVhZChmcmFtZS0+ZG9jdW1lbnQoKS0+dXJsKCkpKSB8 fCBkZWZhdWx0VmFsdWU7CiB9CiAKLWJvb2wgRWRpdG9yQ2xpZW50SW1wbDo6Y2FuUGFzdGUoRnJh bWUqLCBib29sIGRlZmF1bHRWYWx1ZSkgY29uc3QKK2Jvb2wgRWRpdG9yQ2xpZW50SW1wbDo6Y2Fu UGFzdGUoRnJhbWUqIGZyYW1lLCBib29sIGRlZmF1bHRWYWx1ZSkgY29uc3QKIHsKLSAgICByZXR1 cm4gZGVmYXVsdFZhbHVlOworICAgIHJldHVybiAobV93ZWJWaWV3LT5jbGllbnQoKSAmJiBtX3dl YlZpZXctPmNsaWVudCgpLT5jYW5UcmlnZ2VyQ2xpcGJvYXJkV3JpdGUoZnJhbWUtPmRvY3VtZW50 KCktPnVybCgpKSkgfHwgZGVmYXVsdFZhbHVlOwogfQogCiBib29sIEVkaXRvckNsaWVudEltcGw6 OmNhblVuZG8oKSBjb25zdAo= 91024 2011-04-25 16:56:40 -0700 2011-04-26 15:59:17 -0700 Patch bug-59264-20110425165639.patch text/plain 2749 dcheng U3VidmVyc2lvbiBSZXZpc2lvbjogODQ2ODUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKaW5kZXgg NzJkNGIwNTA2NjdjZjMzMmNmY2ZkZGFmNjkzMDdjNmY4ZTg2NTY2YS4uYTkyYTgwODQzMmFlOGMy NGZmMDYyZmRmYzJlMjJjMmU1MTQxNzdkNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKQEAg LTEsMyArMSwxNyBAQAorMjAxMS0wNC0yMiAgRGFuaWVsIENoZW5nICA8ZGNoZW5nQGNocm9taXVt Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBb Y2hyb21pdW1dIEltcGxlbWVudCBFZGl0b3JDbGllbnQ6OmNhbkNvcHlDdXQgYW5kIEVkaXRvckNs aWVudDo6Y2FuUGFzdGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTU5MjY0CisKKyAgICAgICAgKiBwdWJsaWMvV2ViVmlld0NsaWVudC5oOgorICAgICAg ICAoV2ViS2l0OjpXZWJWaWV3Q2xpZW50OjpjYW5UcmlnZ2VyQ2xpcGJvYXJkUmVhZCk6CisgICAg ICAgIChXZWJLaXQ6OldlYlZpZXdDbGllbnQ6OmNhblRyaWdnZXJDbGlwYm9hcmRXcml0ZSk6Cisg ICAgICAgICogc3JjL0VkaXRvckNsaWVudEltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpFZGl0 b3JDbGllbnRJbXBsOjpjYW5Db3B5Q3V0KToKKyAgICAgICAgKFdlYktpdDo6RWRpdG9yQ2xpZW50 SW1wbDo6Y2FuUGFzdGUpOgorCiAyMDExLTA0LTIyICBNaWtoYWlsIE5hZ2Fub3YgIDxtbmFnYW5v dkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgUGF2ZWwgRmVsZG1hbi4KZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZpZXdDbGllbnQuaCBi L1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vcHVibGljL1dlYlZpZXdDbGllbnQuaAppbmRleCA1MTc0 MzUxMzYyNmMwMzk2MjZlNmQ4ZTY2MGRkODY5Y2VlNzQxNzhhLi5iYzc0Y2YzNTFjYzlmMjA0YTE0 MDgxZmVlZWQwMTQ0ZTU0Mjk0NmYzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVt L3B1YmxpYy9XZWJWaWV3Q2xpZW50LmgKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9wdWJs aWMvV2ViVmlld0NsaWVudC5oCkBAIC0xNjQsNiArMTY0LDkgQEAgcHVibGljOgogICAgIHZpcnR1 YWwgdm9pZCBkaWRDaGFuZ2VDb250ZW50cygpIHsgfQogICAgIHZpcnR1YWwgdm9pZCBkaWRFeGVj dXRlQ29tbWFuZChjb25zdCBXZWJTdHJpbmcmIGNvbW1hbmROYW1lKSB7IH0KICAgICB2aXJ0dWFs IHZvaWQgZGlkRW5kRWRpdGluZygpIHsgfQorICAgIHZpcnR1YWwgYm9vbCBjYW5UcmlnZ2VyQ2xp cGJvYXJkUmVhZChjb25zdCBXZWJVUkwmKSB7IHJldHVybiBmYWxzZTsgfQorICAgIHZpcnR1YWwg Ym9vbCBjYW5UcmlnZ2VyQ2xpcGJvYXJkV3JpdGUoY29uc3QgV2ViVVJMJikgeyByZXR1cm4gZmFs c2U7IH0KKwogCiAgICAgLy8gVGhpcyBtZXRob2QgaXMgY2FsbGVkIGluIHJlc3BvbnNlIHRvIFdl YlZpZXcncyBoYW5kbGVJbnB1dEV2ZW50KCkKICAgICAvLyB3aGVuIHRoZSBkZWZhdWx0IGFjdGlv biBmb3IgdGhlIGN1cnJlbnQga2V5Ym9hcmQgZXZlbnQgaXMgbm90CmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL3NyYy9FZGl0b3JDbGllbnRJbXBsLmNwcCBiL1NvdXJjZS9XZWJL aXQvY2hyb21pdW0vc3JjL0VkaXRvckNsaWVudEltcGwuY3BwCmluZGV4IDdjYzhmM2NjOWQ2MGE2 MTJkNzBhMWQ2NTExYzg2YTFlNWQzOTAyYjIuLmE2YmMzYTM5Yzk2YjViYTQ1NzViMzY2MDFiYWE0 Yzk4MjMyOGJlMjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL0VkaXRv ckNsaWVudEltcGwuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL0VkaXRvckNs aWVudEltcGwuY3BwCkBAIC0zMjAsMTQgKzMyMCwxNCBAQCB2b2lkIEVkaXRvckNsaWVudEltcGw6 OmNsZWFyVW5kb1JlZG9PcGVyYXRpb25zKCkKICAgICBtX3JlZG9TdGFjay5jbGVhcigpOwogfQog Ci1ib29sIEVkaXRvckNsaWVudEltcGw6OmNhbkNvcHlDdXQoRnJhbWUqLCBib29sIGRlZmF1bHRW YWx1ZSkgY29uc3QKK2Jvb2wgRWRpdG9yQ2xpZW50SW1wbDo6Y2FuQ29weUN1dChGcmFtZSogZnJh bWUsIGJvb2wgZGVmYXVsdFZhbHVlKSBjb25zdAogewotICAgIHJldHVybiBkZWZhdWx0VmFsdWU7 CisgICAgcmV0dXJuIChtX3dlYlZpZXctPmNsaWVudCgpICYmIG1fd2ViVmlldy0+Y2xpZW50KCkt PmNhblRyaWdnZXJDbGlwYm9hcmRSZWFkKGZyYW1lLT5kb2N1bWVudCgpLT51cmwoKSkpIHx8IGRl ZmF1bHRWYWx1ZTsKIH0KIAotYm9vbCBFZGl0b3JDbGllbnRJbXBsOjpjYW5QYXN0ZShGcmFtZSos IGJvb2wgZGVmYXVsdFZhbHVlKSBjb25zdAorYm9vbCBFZGl0b3JDbGllbnRJbXBsOjpjYW5QYXN0 ZShGcmFtZSogZnJhbWUsIGJvb2wgZGVmYXVsdFZhbHVlKSBjb25zdAogewotICAgIHJldHVybiBk ZWZhdWx0VmFsdWU7CisgICAgcmV0dXJuIChtX3dlYlZpZXctPmNsaWVudCgpICYmIG1fd2ViVmll dy0+Y2xpZW50KCktPmNhblRyaWdnZXJDbGlwYm9hcmRXcml0ZShmcmFtZS0+ZG9jdW1lbnQoKS0+ dXJsKCkpKSB8fCBkZWZhdWx0VmFsdWU7CiB9CiAKIGJvb2wgRWRpdG9yQ2xpZW50SW1wbDo6Y2Fu VW5kbygpIGNvbnN0Cg==