WebKit2:Services menu item to convert selected Simplified/Traditional Chinese Text is not working

[Enrica Casucci]

Repro steps:
1. Launch Safari and navigate to http://www.mozilla.org/editor/midasdemo/
2. Type "简体".
3. Control-click on the first character to bring up the context menu.

Expected

There should be an entry at the bottom of the menu "Convert Selected Simplified Chinese Text"

Actual

The entry is not there.

[Enrica Casucci]

Created attachment 86822 [details]
Patch

This patch adds the support for the services from the Services menu. The context menu part is still missing.

[Alexey Proskuryakov]

Comment on attachment 86822 [details]
Patch

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

> Source/WebCore/editing/mac/EditorMac.mm:219
> +void Editor::readSelectionFromPasteboard(const String& pasteboardName)

Should this be share with WebKit1?

> Source/WebKit2/UIProcess/API/mac/WKView.mm:503
> +// The following three methods are needed to support Mac OS X services.

I would have put a "This method is needed to support Mac OS X Services" before each of the two (?) functions that are required. It's easy to overlook a comment that's position like this.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:540
> +    /*
> +    Frame* coreFrame = core([self _frame]); 
> +    if (!coreFrame) 
> +        return NO; 
> +    if (coreFrame->selection()->isContentRichlyEditable()) 
> +        [self _pasteWithPasteboard:pasteboard allowPlainText:YES]; 
> +    else 
> +        [self _pasteAsPlainTextWithPasteboard:pasteboard]; 
> +     */

Commented out code.

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:167
> +    const double MessageTimeout = 20;

Why capital M? 

I assume that this is in seconds. Seems appropriate to me, but it's strange to see the constant in the function, not at the top of the file.

> LayoutTests/ChangeLog:5
> +        Repeated copy and paste-in-place operation results in increasingly verbose HTML.

This is from another patch.

[Enrica Casucci]

(In reply to comment #2)
> (From update of attachment 86822 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=86822&action=review
> 
> > Source/WebCore/editing/mac/EditorMac.mm:219
> > +void Editor::readSelectionFromPasteboard(const String& pasteboardName)
> 
> Should this be share with WebKit1?
> 
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:503
> > +// The following three methods are needed to support Mac OS X services.
> 
> I would have put a "This method is needed to support Mac OS X Services" before each of the two (?) functions that are required. It's easy to overlook a comment that's position like this.
> 
Done.
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:540
> > +    /*
> > +    Frame* coreFrame = core([self _frame]); 
> > +    if (!coreFrame) 
> > +        return NO; 
> > +    if (coreFrame->selection()->isContentRichlyEditable()) 
> > +        [self _pasteWithPasteboard:pasteboard allowPlainText:YES]; 
> > +    else 
> > +        [self _pasteAsPlainTextWithPasteboard:pasteboard]; 
> > +     */
> 
> Commented out code.
>
My bad.
> > Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:167
> > +    const double MessageTimeout = 20;
> 
> Why capital M? 
> 
> I assume that this is in seconds. Seems appropriate to me, but it's strange to see the constant in the function, not at the top of the file.
> 
I followed how it was done in other places.
> > LayoutTests/ChangeLog:5
> > +        Repeated copy and paste-in-place operation results in increasingly verbose HTML.
> 
> This is from another patch.
Mistake.

[Enrica Casucci]

http://trac.webkit.org/changeset/81898.
Leaving the bug open since the context menu specific portion is still not finished.

[Brady Eidson]

http://trac.webkit.org/changeset/82471 resolved the context menu piece of this.