Bug 223551 - Web Inspector: rename WebInspectorProxy to WebInspectorUIProxy
Summary: Web Inspector: rename WebInspectorProxy to WebInspectorUIProxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-19 23:24 PDT by BJ Burg
Modified: 2021-05-18 14:23 PDT (History)
17 users (show)

See Also:


Attachments
Patch v1.0 (443.24 KB, patch)
2021-03-19 23:43 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v1.3 - for GTK EWS (413.79 KB, patch)
2021-03-22 12:22 PDT, BJ Burg
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch v1.4 - for GTK/Win EWS (413.73 KB, patch)
2021-03-22 13:32 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2021-03-19 23:24:30 PDT
This has bugged me for so long. Might as well fix it.

This makes the -Proxy class have the same prefix as the non-Proxy class.


WebInspectorUI is in WebProcess.
WebInspectorUIProxy is in UIProcess.
Comment 1 BJ Burg 2021-03-19 23:43:24 PDT
Created attachment 423804 [details]
Patch v1.0
Comment 2 EWS Watchlist 2021-03-22 04:48:16 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Carlos Garcia Campos 2021-03-22 04:54:19 PDT
Comment on attachment 423804 [details]
Patch v1.0

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

> Source/WebCore/platform/gtk/po/ar.po:1797
> +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:93
> +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:153

Don't worry about these, they don't even exist. They are old comments. Normally, only POTFILES.in needs to be updated, but WebInspectorProxyGtk.cpp doesn't have translatable strings anymore, so it's not even listed in POTFILES.in
Comment 4 BJ Burg 2021-03-22 10:34:50 PDT
Comment on attachment 423804 [details]
Patch v1.0

Retrying failed GTK EWS builders. I suspect they need a clean build, because Windows is compiling just fine.
Comment 5 BJ Burg 2021-03-22 10:35:15 PDT
(In reply to Carlos Garcia Campos from comment #3)
> Comment on attachment 423804 [details]
> Patch v1.0
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423804&action=review
> 
> > Source/WebCore/platform/gtk/po/ar.po:1797
> > +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:93
> > +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:153
> 
> Don't worry about these, they don't even exist. They are old comments.
> Normally, only POTFILES.in needs to be updated, but WebInspectorProxyGtk.cpp
> doesn't have translatable strings anymore, so it's not even listed in
> POTFILES.in

Heh, okay, I'll revert that part. Thanks for the info!
Comment 6 Don Olmstead 2021-03-22 12:17:22 PDT
Comment on attachment 423804 [details]
Patch v1.0

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

> Source/WebKit/DerivedSources-input.xcfilelist:107
> -$(PROJECT_DIR)/UIProcess/Inspector/RemoteWebInspectorProxy.messages.in
> -$(PROJECT_DIR)/UIProcess/Inspector/WebInspectorProxy.messages.in
> +$(PROJECT_DIR)/UIProcess/Inspector/RemoteWebInspectorUIProxy.messages.in
> +$(PROJECT_DIR)/UIProcess/Inspector/WebInspectorUIProxy.messages.in

I don't see a corresponding change to the CMakeLists.txt

https://github.com/WebKit/WebKit/blob/main/Source/WebKit/CMakeLists.txt#L205-L206
Comment 7 BJ Burg 2021-03-22 12:22:01 PDT
Created attachment 423918 [details]
Patch v1.3 - for GTK EWS
Comment 8 Radar WebKit Bug Importer 2021-03-22 12:35:12 PDT
<rdar://problem/75703828>
Comment 9 BJ Burg 2021-03-22 13:32:14 PDT
Created attachment 423930 [details]
Patch v1.4 - for GTK/Win EWS
Comment 10 Devin Rousso 2021-03-22 15:51:35 PDT
Comment on attachment 423930 [details]
Patch v1.4 - for GTK/Win EWS

rs=me, always wanted to do this :P

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

>>> Source/WebCore/platform/gtk/po/ar.po:1797
>>> +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:153
>> 
>> Don't worry about these, they don't even exist. They are old comments. Normally, only POTFILES.in needs to be updated, but WebInspectorProxyGtk.cpp doesn't have translatable strings anymore, so it's not even listed in POTFILES.in
> 
> Heh, okay, I'll revert that part. Thanks for the info!

Can we remove these files then (or at least these parts of these files)?
Comment 11 EWS 2021-03-22 16:26:07 PDT
Committed r274815: <https://commits.webkit.org/r274815>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423930 [details].
Comment 12 Carlos Garcia Campos 2021-03-23 01:43:52 PDT
(In reply to Devin Rousso from comment #10)
> Comment on attachment 423930 [details]
> Patch v1.4 - for GTK/Win EWS
> 
> rs=me, always wanted to do this :P
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423804&action=review
> 
> >>> Source/WebCore/platform/gtk/po/ar.po:1797
> >>> +#: ../../../WebKit2/UIProcess/gtk/WebInspectorUIProxyGtk.cpp:153
> >> 
> >> Don't worry about these, they don't even exist. They are old comments. Normally, only POTFILES.in needs to be updated, but WebInspectorProxyGtk.cpp doesn't have translatable strings anymore, so it's not even listed in POTFILES.in
> > 
> > Heh, okay, I'll revert that part. Thanks for the info!
> 
> Can we remove these files then (or at least these parts of these files)?

No, those are valid translation files. I think those comments are just hints to know where the translatable string come from in the code, but in practice it doesn't really matter if it's part of a different file, the translation will be done in any case.
Comment 13 Tim Horton 2021-04-08 01:25:07 PDT
Comment on attachment 423930 [details]
Patch v1.4 - for GTK/Win EWS

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

> Source/WebKit/UIProcess/API/C/mac/WKInspectorPrivateMac.h:44
> -WK_EXPORT @interface WKWebInspectorProxyObjCAdapter : NSObject <NSWindowDelegate>
> +WK_EXPORT @interface WKWebInspectorUIProxyObjCAdapter : NSObject <NSWindowDelegate>

I think we need to revert this change, since it breaks SafariForWebKitDevelopment.
Comment 14 BJ Burg 2021-04-08 09:01:34 PDT
Comment on attachment 423930 [details]
Patch v1.4 - for GTK/Win EWS

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

>> Source/WebKit/UIProcess/API/C/mac/WKInspectorPrivateMac.h:44
>> +WK_EXPORT @interface WKWebInspectorUIProxyObjCAdapter : NSObject <NSWindowDelegate>
> 
> I think we need to revert this change, since it breaks SafariForWebKitDevelopment.

Alternative approach, define WKWebInspectorProxyObjCAdapter as a subclass of WKWebInpectorUIProxyObjCAdapter with nothing extra, and export it?
Comment 15 Tim Horton 2021-04-08 12:20:42 PDT
(In reply to BJ Burg from comment #14)
> Comment on attachment 423930 [details]
> Patch v1.4 - for GTK/Win EWS
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423930&action=review
> 
> >> Source/WebKit/UIProcess/API/C/mac/WKInspectorPrivateMac.h:44
> >> +WK_EXPORT @interface WKWebInspectorUIProxyObjCAdapter : NSObject <NSWindowDelegate>
> > 
> > I think we need to revert this change, since it breaks SafariForWebKitDevelopment.
> 
> Alternative approach, define WKWebInspectorProxyObjCAdapter as a subclass of
> WKWebInpectorUIProxyObjCAdapter with nothing extra, and export it?

r=me :)
Comment 16 Alex Christensen 2021-05-18 14:23:52 PDT
r277678