[webkit-reviews] review granted: [Bug 240044] Remove deprecated 'JavaEnabled' feature flag and related code : [Attachment 458765] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 240044: Remove deprecated 'JavaEnabled' feature flag and related code
https://bugs.webkit.org/show_bug.cgi?id=240044

Attachment 458765: Patch

https://bugs.webkit.org/attachment.cgi?id=458765&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 458765
  --> https://bugs.webkit.org/attachment.cgi?id=458765
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
> + at 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
>  */
> - at property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled;
> + at 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.