WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
52417
Add EditorClient callbacks to override isDOMPasteAllowed and javaScriptCanAccessClipboard
https://bugs.webkit.org/show_bug.cgi?id=52417
Summary
Add EditorClient callbacks to override isDOMPasteAllowed and javaScriptCanAcc...
Ryosuke Niwa
Reported
2011-01-13 17:51:07 PST
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.
Attachments
proof of concept on Mac port
(16.16 KB, patch)
2011-01-13 18:12 PST
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Patch
(23.79 KB, patch)
2011-02-07 23:07 PST
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Fixed per Adam's comment
(24.99 KB, patch)
2011-02-14 22:29 PST
,
Ryosuke Niwa
abarth
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2011-01-13 18:12:28 PST
Created
attachment 78881
[details]
proof of concept on Mac port
Ryosuke Niwa
Comment 2
2011-01-13 21:46:43 PST
Comment on
attachment 78881
[details]
proof of concept on Mac port 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?
Alexey Proskuryakov
Comment 3
2011-01-13 23:24:06 PST
> + 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.
Adam Barth
Comment 4
2011-01-14 23:48:18 PST
We discussed this on IRC. Our current thinking is to add SecurityOrigin::canPaste style functions and use a whitelist.
Ryosuke Niwa
Comment 5
2011-02-07 21:29:11 PST
After another discussion with abarth and othermaciej, I think we're going back to the original plan of adding an method to EditorClient.
Ryosuke Niwa
Comment 6
2011-02-07 23:07:20 PST
Created
attachment 81592
[details]
Patch
Ryosuke Niwa
Comment 7
2011-02-14 18:30:04 PST
Hi, could someone review my patch? This patch or some alternative mechanism to control copy & paste per origin is required for a Chromium feature.
Adam Barth
Comment 8
2011-02-14 18:50:08 PST
Comment on
attachment 81592
[details]
Patch 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.
Ryosuke Niwa
Comment 9
2011-02-14 22:29:34 PST
Created
attachment 82415
[details]
Fixed per Adam's comment
Adam Barth
Comment 10
2011-02-14 22:38:17 PST
Comment on
attachment 82415
[details]
Fixed per Adam's comment 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.
Ryosuke Niwa
Comment 11
2011-02-14 22:55:50 PST
Thanks for the review! (In reply to
comment #10
)
> (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.
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.
Ryosuke Niwa
Comment 12
2011-02-15 00:22:03 PST
Committed
r78532
: <
http://trac.webkit.org/changeset/78532
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug