Bug 191854 - Remove the need for LocalizedStringsWPE.cpp
Summary: Remove the need for LocalizedStringsWPE.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-20 00:02 PST by Christopher Reid
Modified: 2018-11-20 14:10 PST (History)
7 users (show)

See Also:


Attachments
patch (7.01 KB, patch)
2018-11-20 00:52 PST, Christopher Reid
no flags Details | Formatted Diff | Diff
patch (7.45 KB, patch)
2018-11-20 00:56 PST, Christopher Reid
no flags Details | Formatted Diff | Diff
rebased patch (7.45 KB, patch)
2018-11-20 01:18 PST, Christopher Reid
no flags Details | Formatted Diff | Diff
patch (7.44 KB, patch)
2018-11-20 01:59 PST, Christopher Reid
no flags Details | Formatted Diff | Diff
Patch for landing (7.88 KB, patch)
2018-11-20 13:42 PST, Christopher Reid
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Reid 2018-11-20 00:02:49 PST
These functions can be shared between ports.
Comment 1 Christopher Reid 2018-11-20 00:52:09 PST
Created attachment 355319 [details]
patch
Comment 2 Christopher Reid 2018-11-20 00:56:03 PST
Created attachment 355320 [details]
patch
Comment 3 Christopher Reid 2018-11-20 01:18:52 PST
Created attachment 355324 [details]
rebased patch
Comment 4 Christopher Reid 2018-11-20 01:59:13 PST
Created attachment 355326 [details]
patch
Comment 5 Christopher Reid 2018-11-20 02:01:53 PST
I wanted to check with Apple Win if there's any objection to replacing "Search with Google" to a more generic "Search the Web".
Comment 6 Don Olmstead 2018-11-20 10:23:24 PST
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.
Comment 7 Christopher Reid 2018-11-20 11:16:23 PST
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.
Comment 8 Don Olmstead 2018-11-20 12:09:32 PST
(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.
Comment 9 Christopher Reid 2018-11-20 13:42:47 PST
Created attachment 355362 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2018-11-20 14:09:30 PST
Comment on attachment 355362 [details]
Patch for landing

Clearing flags on attachment: 355362

Committed r238406: <https://trac.webkit.org/changeset/238406>
Comment 11 WebKit Commit Bot 2018-11-20 14:09:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-11-20 14:10:27 PST
<rdar://problem/46190457>