WebKit Bugzilla
You need to log in before you can comment on or make changes to this bug.
Right now, the permission to execute copy, cut, paste is dictated by isDOMPasteAllowed and javaScriptCanAccessClipboard, which are called by supportedCopyCut and supportedPaste. However, this won't allow fine-grained control such as allowing copy, cut, paste for certain origins. I'd like to make these EditorClient callbacks so that EditorClient can decide whether or not copy, cut, & paste is supported or not per frame.
Created an attachment (id=78881) [details] proof of concept on Mac port
(From update of attachment 78881 [details]) View in context: https://bugs.webkit.org/attachment.cgi?id=78881&action=review > WebKit2/WebProcess/WebPage/WebPage.cpp:1189 > - settings->setDOMPasteAllowed(store.getBoolValueForKey(WebPreferencesKey::domPasteAllowedKey())); > + > + m_isCopyCutEnabled = store.getBoolValueForKey(WebPreferencesKey::domPasteAllowedKey()); I don't know what's the correct thing to do for WebKit2. Could someone familiar with WebKit2 comment on what I should be doing here? Enrica?
> + m_isCopyCutEnabled = store.getBoolValueForKey(WebPreferencesKey::domPasteAllowedKey()); This looks good WebKit2-wise, but not so good logically. Why does m_isCopyCutEnabled have to do with domPasteAllowed? These new client calls have extremely generic names that hide their meaning. Compare to javaScriptCanAccessClipboard() and isDOMPasteAllowed(). + WebPreferences* pref = [m_webView preferences]; No abbr.? I'm not sure if giving some frames extra privileges is good security architecture. Adam will hopefully tell us if it's not! Perhaps this should be a page group option instead.
We discussed this on IRC. Our current thinking is to add SecurityOrigin::canPaste style functions and use a whitelist.
After another discussion with abarth and othermaciej, I think we're going back to the original plan of adding an method to EditorClient.
Created an attachment (id=81592) [details] Patch
Hi, could someone review my patch? This patch or some alternative mechanism to control copy & paste per origin is required for a Chromium feature.
(From update of attachment 81592 [details]) View in context: https://bugs.webkit.org/attachment.cgi?id=81592&action=review > Source/WebCore/editing/EditorCommand.cpp:1129 > + EditorClient* client = frame->editor()->client(); > + if (client && client->canCopyCut()) > + return true; It's strange that if the client returns false from canCopyCut, the web page might still be able to copy or cut. That's why I recommended the design we use for allowPlugins where we pass the default as a parameter. That way its easy for the client to accept the default and also easy for the client to have the final say.
Created an attachment (id=82415) [details] Fixed per Adam's comment
(From update of attachment 82415 [details]) View in context: https://bugs.webkit.org/attachment.cgi?id=82415&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:256 > + notImplemented(); These probably don't need notImplemented(). These are perfectly fine implementations. > Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h:78 > + virtual bool canCopyCut(bool defaultValue) const; > + virtual bool canPaste(bool defaultValue) const; Are these the same semantically as the functions below? If not, consider renaming them to allowPaste, etc.
Thanks for the review! (In reply to comment #10) > (From update of attachment 82415 [details] [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82415&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:256 > > + notImplemented(); > > These probably don't need notImplemented(). These are perfectly fine implementations. Ok, will remove. > > Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h:78 > > + virtual bool canCopyCut(bool defaultValue) const; > > + virtual bool canPaste(bool defaultValue) const; > > Are these the same semantically as the functions below? If not, consider renaming them to allowPaste, etc. Semantics depends on each implementation because some client might always return true/false to indicate that copy/cut/paste are always possible. canUndo/canRedo are usually implemented to return true iff we have objects in the undo stack but clients are free to access control undo/redo per origin via these two functions as well.
Committed r78532: <http://trac.webkit.org/changeset/78532>