Bug 45494 - Paste should be implemented in WebCore like Copy and Cut for Mac also
Summary: Paste should be implemented in WebCore like Copy and Cut for Mac also
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords:
Depends on:
Blocks: 42317
  Show dependency treegraph
 
Reported: 2010-09-09 15:06 PDT by Enrica Casucci
Modified: 2010-09-14 04:56 PDT (History)
5 users (show)

See Also:


Attachments
Patch (34.31 KB, patch)
2010-09-09 15:31 PDT, Enrica Casucci
no flags Details | Formatted Diff | Diff
Patch2 (34.26 KB, patch)
2010-09-09 16:18 PDT, Enrica Casucci
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2010-09-09 15:06:57 PDT
This bug tracks the refactoring work needed to support paste in WebKit2.
Comment 1 Enrica Casucci 2010-09-09 15:31:26 PDT
Created attachment 67102 [details]
Patch

I'm still trying to build this on Windows.
Comment 2 Early Warning System Bot 2010-09-09 15:47:46 PDT
Attachment 67102 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3912393
Comment 3 Ryosuke Niwa 2010-09-09 15:52:28 PDT
Comment on attachment 67102 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67102&action=prettypatch

> WebCore/editing/mac/EditorMac.mm:86
> -
> +    
nit: please don't add spaces in blank lines.

> WebCore/platform/mac/PasteboardMac.mm:381
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:397
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:422
> +       
spaces.

> WebCore/platform/mac/PasteboardMac.mm:436
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:444
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:468
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:502
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:506
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:510
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:514
> +        
spaces.

> WebCore/platform/mac/PasteboardMac.mm:518
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:524
> +    
spaces.

> WebCore/platform/mac/PasteboardMac.mm:530
> +    
spaces.

> WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:326
> +{      
nit: there are spaes after {

> WebKit/mac/WebCoreSupport/WebEditorClient.mm:367
> +    
spaces.

> WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1021
> +bool WebFrameLoaderClient::canShowMIMETypeAsHTML(const String& MIMEType) const
I'm not particularly familiar with this part of the code but why do we need to add new function as supposed to overriding canShowMIMEType?
It'll be very helpful if you could document that in the change log or add some comment in the base class.
Comment 4 Enrica Casucci 2010-09-09 16:11:04 PDT
(In reply to comment #3)
Fixed all the spaces.

> > WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1021
> > +bool WebFrameLoaderClient::canShowMIMETypeAsHTML(const String& MIMEType) const
> I'm not particularly familiar with this part of the code but why do we need to add new function as supposed to overriding canShowMIMEType?
> It'll be very helpful if you could document that in the change log or add some comment in the base class.

I've changed this to be platform specific and it should be Mac only. This function returns true only if we can show the gives MIMEType as HTML.
Comment 5 Enrica Casucci 2010-09-09 16:18:42 PDT
Created attachment 67111 [details]
Patch2

Remvoed blanks. Made canShowMIMETypeasHTML platform specific.
Comment 6 Sam Weinig 2010-09-10 14:25:22 PDT
Comment on attachment 67111 [details]
Patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=67111&action=prettypatch

> WebCore/loader/FrameLoaderClient.h:209
> +#if PLATFORM(MAC)
> +        virtual bool canShowMIMETypeAsHTML(const String& MIMEType) const = 0;
> +#endif
This doesn't have to be mac specific.

> WebCore/page/EditorClient.h:156
> +    virtual DocumentFragment* documentFromAttributedString(NSAttributedString*, Vector<ArchiveResource*>&) = 0;
This should probably be called documentFragmentFromAttributedString.

> WebCore/platform/mac/PasteboardMac.mm:441
> +PassRefPtr<DocumentFragment> Pasteboard::documentFragmentFromPasteboard(Frame* frame, PassRefPtr<Range> context, bool allowPlainText)
> +{
I think this function should be merged into Pasteboard::documentFragment.


r+ with this changes.
Comment 7 Enrica Casucci 2010-09-13 12:22:32 PDT
http://trac.webkit.org/changeset/67403
Comment 8 Lucas De Marchi 2010-09-14 04:56:40 PDT
(In reply to comment #6)
> (From update of attachment 67111 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67111&action=prettypatch
> 
> > WebCore/loader/FrameLoaderClient.h:209
> > +#if PLATFORM(MAC)
> > +        virtual bool canShowMIMETypeAsHTML(const String& MIMEType) const = 0;
> > +#endif
> This doesn't have to be mac specific.

The patch applied contains this #if, which breaks EFL port. Please, see https://bugs.webkit.org/show_bug.cgi?id=45728