WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug