RESOLVED FIXED 56975
WebKit2:Services menu item to convert selected Simplified/Traditional Chinese Text is not working
https://bugs.webkit.org/show_bug.cgi?id=56975
Summary WebKit2:Services menu item to convert selected Simplified/Traditional Chinese...
Enrica Casucci
Reported 2011-03-23 16:35:55 PDT
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.
Attachments
Patch (10.51 KB, patch)
2011-03-24 13:42 PDT, Enrica Casucci
ap: review+
Enrica Casucci
Comment 1 2011-03-24 13:42:53 PDT
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 2 2011-03-24 14:01:07 PDT
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
Comment 3 2011-03-24 14:17:49 PDT
(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
Comment 4 2011-03-24 14:23:18 PDT
http://trac.webkit.org/changeset/81898. Leaving the bug open since the context menu specific portion is still not finished.
Brady Eidson
Comment 5 2011-04-11 13:42:43 PDT
http://trac.webkit.org/changeset/82471 resolved the context menu piece of this.
Note You need to log in before you can comment on or make changes to this bug.