Bug 191854 – Remove the need for LocalizedStringsWPE.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Christopher Reid]

These functions can be shared between ports.

[Christopher Reid]

Created attachment 355319 [details]
patch

[Christopher Reid]

Created attachment 355320 [details]
patch

[Christopher Reid]

Created attachment 355324 [details]
rebased patch

[Christopher Reid]

Created attachment 355326 [details]
patch

[Christopher Reid]

I wanted to check with Apple Win if there's any objection to replacing "Search with Google" to a more generic "Search the Web".

[Don Olmstead]

Comment on attachment 355326 [details]
patch

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

I don't think it matters for AppleWin since its just used for embedding in apps.

> Source/WebCore/en.lproj/Localizable.strings:602
> -/* Search with Google context menu item */
> -"Search with Google" = "Search with Google";
> +/* Search the Web context menu item */
> +"Search the Web" = "Search the Web";

I have a feeling we might want to keep the original and add the new one. I'm not sure how this will affect XCode builds.

[Christopher Reid]

Comment on attachment 355326 [details]
patch

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

> Source/WebCore/ChangeLog:13
> +        of String::fromUTF8. Move that to LocalizedString.cpp as the default implement

Just noticed this typo, will change implement to implementation before landing

>> Source/WebCore/en.lproj/Localizable.strings:602
>> +"Search the Web" = "Search the Web";
> 
> I have a feeling we might want to keep the original and add the new one. I'm not sure how this will affect XCode builds.

That string seems to only be used in windows CF ports. COCOA is using the "Search with %@" string. I'm not certain about the NSPerformService(@"Search With Google", pasteboard); call in WebKitLegacy's WebView.mm but it looks like it's only used for invoking a context menu action and not for the context menu text so it doesn't seem to be doing localization. I'm not familiar enough with NSPerformService to confirm whether that's true or not. If it is the case, I don't see much value in keeping the original string around.

[Don Olmstead]

(In reply to Christopher Reid from comment #7)
> Comment on attachment 355326 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355326&action=review
> 
> > Source/WebCore/ChangeLog:13
> > +        of String::fromUTF8. Move that to LocalizedString.cpp as the default implement
> 
> Just noticed this typo, will change implement to implementation before
> landing
> 
> >> Source/WebCore/en.lproj/Localizable.strings:602
> >> +"Search the Web" = "Search the Web";
> > 
> > I have a feeling we might want to keep the original and add the new one. I'm not sure how this will affect XCode builds.
> 
> That string seems to only be used in windows CF ports. COCOA is using the
> "Search with %@" string. I'm not certain about the NSPerformService(@"Search
> With Google", pasteboard); call in WebKitLegacy's WebView.mm but it looks
> like it's only used for invoking a context menu action and not for the
> context menu text so it doesn't seem to be doing localization. I'm not
> familiar enough with NSPerformService to confirm whether that's true or not.
> If it is the case, I don't see much value in keeping the original string
> around.

Ok then I guess this is good to go after you fix your typo.

[Christopher Reid]

Created attachment 355362 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 355362 [details]
Patch for landing

Clearing flags on attachment: 355362

Committed r238406: <https://trac.webkit.org/changeset/238406>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46190457>