WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
84766
REGRESSION (
r109022
) Safari not placing service data on pasteboard
https://bugs.webkit.org/show_bug.cgi?id=84766
Summary
REGRESSION (r109022) Safari not placing service data on pasteboard
Enrica Casucci
Reported
2012-04-24 14:08:17 PDT
This is a regression introduced during one of the changes to move access to the pasteboard out of the WebProcess. The support for OS X Services requires that all the write operation to the pasteboard are made synchronously. <
rdar://problem/11085756
>
Attachments
Patch
(15.80 KB, patch)
2012-04-24 14:19 PDT
,
Enrica Casucci
ap
: review+
ap
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Enrica Casucci
Comment 1
2012-04-24 14:19:07 PDT
Created
attachment 138642
[details]
Patch
WebKit Review Bot
Comment 2
2012-04-24 14:23:57 PDT
Attachment 138642
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit2/WebProcess/WebPage/WebPage.h:414: The parameter name "handle" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Enrica Casucci
Comment 3
2012-04-24 14:31:30 PDT
(In reply to
comment #2
)
>
Attachment 138642
[details]
did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 > Source/WebKit2/WebProcess/WebPage/WebPage.h:414: The parameter name "handle" adds no information, so it should be removed. [readability/parameter_name] [5] > Total errors found: 1 in 11 files > > > If any of these errors are false positives, please file a bug against check-webkit-style.
Already fixed.
Jeff Miller
Comment 4
2012-04-24 15:17:19 PDT
Comment on
attachment 138642
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=138642&action=review
> Source/WebCore/ChangeLog:8 > + the pasteboard occurr synchronously. This behavior was changed with
r109022
.
Type - occurr
> Source/WebKit2/ChangeLog:8 > + the pasteboard occurr synchronously. This behavior was changed with
r109022
.
Ditto.
Jeff Miller
Comment 5
2012-04-24 15:17:47 PDT
(In reply to
comment #4
)
> (From update of
attachment 138642
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=138642&action=review
> > > Source/WebCore/ChangeLog:8 > > + the pasteboard occurr synchronously. This behavior was changed with
r109022
. > > Type - occurr
I meant typo, not type. :)
> > > Source/WebKit2/ChangeLog:8 > > + the pasteboard occurr synchronously. This behavior was changed with
r109022
. > > Ditto.
Alexey Proskuryakov
Comment 6
2012-04-24 15:25:24 PDT
Comment on
attachment 138642
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=138642&action=review
> Source/WebCore/editing/mac/EditorMac.mm:33 > #import "DocumentFragment.h" > +#import "DOMRangeInternal.h"
This is out of order, style bit should have complained.
> Source/WebCore/editing/mac/EditorMac.mm:50 > #import "htmlediting.h" > +#import "WebNSAttributedStringExtras.h"
This too - upper case comes first in ASCII order.
> Source/WebCore/editing/mac/EditorMac.mm:318 > +PassRefPtr<SharedBuffer> Editor::getBufferSelectionForPasteboard(const String& pasteboardType)
Did this code come from WebKit1? Can it be deleted there now? This most definitely doesn't look like something we want to maintain in two places forever.
> Source/WebKit2/UIProcess/WebPageProxy.h:473 > + PassRefPtr<WebCore::SharedBuffer> getBufferSelectionForPasteboard(const String& pasteboardType);
I'm not sure if I can parse "buffer selection". Maybe "PasteboardDataFromSelection"? Or "SerializedAttributedStringFromSelection"? This applies to the whole cascade of functions, of course, not just this proxy wrapper.
> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:-263 > - if (!isValid()) > - return false;
Why is this not necessary any more?
> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:278 > + return SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), size);
Style nit: star goes with the type.
Enrica Casucci
Comment 7
2012-04-24 15:42:29 PDT
(In reply to
comment #5
)
> (In reply to
comment #4
) > > (From update of
attachment 138642
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=138642&action=review
> > > > > Source/WebCore/ChangeLog:8 > > > + the pasteboard occurr synchronously. This behavior was changed with
r109022
. > > > > Type - occurr > > I meant typo, not type. :) > > > > > > Source/WebKit2/ChangeLog:8 > > > + the pasteboard occurr synchronously. This behavior was changed with
r109022
. > > > > Ditto.
I'll fix it. Thanks! :-)
Enrica Casucci
Comment 8
2012-04-24 15:45:45 PDT
(In reply to
comment #6
)
> (From update of
attachment 138642
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=138642&action=review
> > > Source/WebCore/editing/mac/EditorMac.mm:33 > > #import "DocumentFragment.h" > > +#import "DOMRangeInternal.h" > > This is out of order, style bit should have complained. > > > Source/WebCore/editing/mac/EditorMac.mm:50 > > #import "htmlediting.h" > > +#import "WebNSAttributedStringExtras.h" > > This too - upper case comes first in ASCII order.
Will do.
> > > Source/WebCore/editing/mac/EditorMac.mm:318 > > +PassRefPtr<SharedBuffer> Editor::getBufferSelectionForPasteboard(const String& pasteboardType) > > Did this code come from WebKit1? Can it be deleted there now?
> > This most definitely doesn't look like something we want to maintain in two places forever. >
No, this code comes from PasteboardMac. I'll refactor it, to avoid duplicates.
> > Source/WebKit2/UIProcess/WebPageProxy.h:473 > > + PassRefPtr<WebCore::SharedBuffer> getBufferSelectionForPasteboard(const String& pasteboardType); > > I'm not sure if I can parse "buffer selection". Maybe "PasteboardDataFromSelection"? Or "SerializedAttributedStringFromSelection"?
I think I'll go for dataFromSelection and stringFromSelection.
> > This applies to the whole cascade of functions, of course, not just this proxy wrapper. > > > Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:-263 > > - if (!isValid()) > > - return false; >
Removed by accident. Thanks for noticing.
> Why is this not necessary any more? > > > Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:278 > > + return SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), size); > > Style nit: star goes with the type.
Sure.
Alexey Proskuryakov
Comment 9
2012-04-24 15:57:20 PDT
> I think I'll go for dataFromSelection and stringFromSelection.
I'm still not sure if that would be clear enough later - when there is a SharedData buffer, it would be helpful to have names explain format for the buffer.
> Removed by accident. Thanks for noticing.
Please add it to the new function as well.
Enrica Casucci
Comment 10
2012-04-24 17:25:23 PDT
http://trac.webkit.org/changeset/115145
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