We removed support for Java Plug-Ins in macOS 10.15, but never removed the preference. To reduce code complexity and build times we should remove this unused preference, leaving non-functional stubs to avoid breaking binaries that expect to call their accessor methods.
Created attachment 458765 [details] Patch
Comment on attachment 458765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458765&action=review > Source/WebCore/loader/SubframeLoader.cpp:132 > + if (MIMETypeRegistry::isJavaAppletMIMEType(mimeType)) > + return false; I understand that this preserves behavior. But do we need this? Why exactly? I think we don’t. > Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:106 > +// Defaults to false. Comment could say something more specific like "does nothing, always returns false". > Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:108 > +WK_EXPORT void WKPreferencesSetJavaEnabled(WKPreferencesRef preferences, bool javaEnabled) WK_C_API_DEPRECATED; > +WK_EXPORT bool WKPreferencesGetJavaEnabled(WKPreferencesRef preferences) WK_C_API_DEPRECATED; Do we need this? Can we just remove it instead? > Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:585 > +WK_EXPORT void WKPreferencesSetJavaEnabledForLocalFiles(WKPreferencesRef preferences, bool javaEnabled) WK_C_API_DEPRECATED; > +WK_EXPORT bool WKPreferencesGetJavaEnabledForLocalFiles(WKPreferencesRef preferences) WK_C_API_DEPRECATED; Do we need this? Can we just remove it instead? > Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm:1641 > #endif > > + > +#if PLATFORM(MAC) No need for the extra blank line. > Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:236 > +@property (nonatomic, setter=_setJavaEnabledForLocalFiles:) BOOL _javaEnabledForLocalFiles WK_API_DEPRECATED("Java is no longer supported", macos(10.13.4, 10.15)); Do we need this? Can we just remove it instead? > Source/WebKitLegacy/mac/WebView/WebPreferenceKeysPrivate.h:293 > +#define WebKitJavaEnabledPreferenceKey @"WebKitJavaEnabled" Why do we need this? I suggest we remove it. > Source/WebKitLegacy/mac/WebView/WebPreferences.h:172 > /*! > @property javaEnabled > */ > -@property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled; > +@property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled WEBKIT_DEPRECATED_MAC(10_3, 10_15); I suggest we have the comment say this does nothing and always returns false. > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3474 > +- (BOOL)isJavaEnabled > +{ > + return NO; > +} > + > +- (void)setJavaEnabled:(BOOL)flag > +{ > +} Why move this to the bottom of the file instead of changing it in place? > Tools/TestWebKitAPI/Tests/WebKit/WKPreferences.cpp:84 > - EXPECT_TRUE(WKPreferencesGetJavaEnabled(preference)); > + EXPECT_FALSE(WKPreferencesGetJavaEnabled(preference)); I suggest we just remove this.
Comment on attachment 458765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458765&action=review >> Source/WebCore/loader/SubframeLoader.cpp:132 >> + return false; > > I understand that this preserves behavior. But do we need this? Why exactly? I think we don’t. I'll just delete it and confirm that passes all tests. My only hesitation is that this method returns 'true' by default, which might give us bad behavior if we just delete. >> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:106 >> +// Defaults to false. > > Comment could say something more specific like "does nothing, always returns false". Will do! >> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:108 >> +WK_EXPORT bool WKPreferencesGetJavaEnabled(WKPreferencesRef preferences) WK_C_API_DEPRECATED; > > Do we need this? Can we just remove it instead? I'll confirm we don't use it anywhere. I didn't do a thorough search, so I'll double check and then remove. I'm a little worried about removing it since it's API. >> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:585 >> +WK_EXPORT bool WKPreferencesGetJavaEnabledForLocalFiles(WKPreferencesRef preferences) WK_C_API_DEPRECATED; > > Do we need this? Can we just remove it instead? I'll bet we can remove both of these, since they are SPI. >> Source/WebKit/UIProcess/API/Cocoa/WKPreferences.mm:1641 >> +#if PLATFORM(MAC) > > No need for the extra blank line. Will fix. >> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:236 >> +@property (nonatomic, setter=_setJavaEnabledForLocalFiles:) BOOL _javaEnabledForLocalFiles WK_API_DEPRECATED("Java is no longer supported", macos(10.13.4, 10.15)); > > Do we need this? Can we just remove it instead? I'll confirm and remove. >> Source/WebKitLegacy/mac/WebView/WebPreferenceKeysPrivate.h:293 >> +#define WebKitJavaEnabledPreferenceKey @"WebKitJavaEnabled" > > Why do we need this? I suggest we remove it. Will do. >> Source/WebKitLegacy/mac/WebView/WebPreferences.h:172 >> +@property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled WEBKIT_DEPRECATED_MAC(10_3, 10_15); > > I suggest we have the comment say this does nothing and always returns false. Will do. >> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3474 >> +} > > Why move this to the bottom of the file instead of changing it in place? I moved it to the 'WKPreferences (WKPrivateDeprecated)' category, along with the other deprecated items. >> Tools/TestWebKitAPI/Tests/WebKit/WKPreferences.cpp:84 >> + EXPECT_FALSE(WKPreferencesGetJavaEnabled(preference)); > > I suggest we just remove this. Will do.
Created attachment 458767 [details] Patch for landing
Patch 458767 does not build
Created attachment 458803 [details] Patch for landing
Tools/Scripts/svn-apply failed to apply attachment 458803 [details] to trunk. Please resolve the conflicts and upload a new patch.
(In reply to EWS from comment #7) > Tools/Scripts/svn-apply failed to apply attachment 458803 [details] to trunk. > Please resolve the conflicts and upload a new patch. Huge EWS fail: "patch: **** Can't create file /var/folders/33/15r3ggyd4fb52p4t38_rbd9w0000gn/T//pp0VQ5bq : No space left on device " We should recognize space failures and not mark a patch as broken when that happens.
Committed r293785 (250264@main): <https://commits.webkit.org/250264@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458803 [details].
<rdar://problem/92750902>
Comment on attachment 458765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458765&action=review >>> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:108 >>> +WK_EXPORT bool WKPreferencesGetJavaEnabled(WKPreferencesRef preferences) WK_C_API_DEPRECATED; >> >> Do we need this? Can we just remove it instead? > > I'll confirm we don't use it anywhere. I didn't do a thorough search, so I'll double check and then remove. I'm a little worried about removing it since it's API. Despite the name of the directory this is in, it is absolutely *not* API on Apple platforms. Our C interface for modern WebKit has only ever been SPI and I’m pretty sure we don’t ever plan to make it API. I suppose on non-Apple platforms it might be API, so if those need this to stay around, I understand why you might not remove it.