Bug 224441

Created attachment 425763 [details] Patch

Disabling plugins seems like a good idea, but I am not totally clear on the motivation for disabling replacement. What is the goal with disabling replacement and does it have to be linked to this (e.g. do we only do replacement if plugins are enabled?)

Comment on attachment 425763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425763&action=review > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:903 > - (void)setPlugInsEnabled:(BOOL)flag > { > - [self _setBoolValue: flag forKey: WebKitPluginsEnabledPreferenceKey]; > } This seems like a step beyond deprecation. This makes it so that apps that enable the feature won't work.

(In reply to Sam Weinig from comment #2) > Disabling plugins seems like a good idea, but I am not totally clear on the > motivation for disabling replacement. What is the goal with disabling > replacement and does it have to be linked to this (e.g. do we only do > replacement if plugins are enabled?) The replacement code is related to the existence of plugins. We could do it in a separate patch, but we also want to do it right now so I didn't see a reason to. It is old code that exists to be compatible with websites of more than a decade ago, which we believe have updated since Flash support has waned. (In reply to Sam Weinig from comment #3) > Comment on attachment 425763 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=425763&action=review > > > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:903 > > - (void)setPlugInsEnabled:(BOOL)flag > > { > > - [self _setBoolValue: flag forKey: WebKitPluginsEnabledPreferenceKey]; > > } > > This seems like a step beyond deprecation. This makes it so that apps that > enable the feature won't work. That is correct. WebPreferences is already deprecated. We will look at seed feedback, but we intend to remove all our NPAPI code if this goes smoothly.

After some internal discussion, we have decided to proceed with this, but split it into three changes: 1. Changing WKWebView to be unable to load plug ins. 2. Changing WebView to be unable to load plug ins. 3. Disabling the plug in replacements.

(In reply to Alex Christensen from comment #4) > (In reply to Sam Weinig from comment #2) > > Disabling plugins seems like a good idea, but I am not totally clear on the > > motivation for disabling replacement. What is the goal with disabling > > replacement and does it have to be linked to this (e.g. do we only do > > replacement if plugins are enabled?) > The replacement code is related to the existence of plugins. We could do it > in a separate patch, but we also want to do it right now so I didn't see a > reason to. It is old code that exists to be compatible with websites of > more than a decade ago, which we believe have updated since Flash support > has waned. > Seems like it would be worth keeping around to me. Keeping old sites that use things like QuickTime seems like a worthy goal. I think at the very least it should be in its own change so that it can be backed out separately. > (In reply to Sam Weinig from comment #3) > > Comment on attachment 425763 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=425763&action=review > > > > > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:903 > > > - (void)setPlugInsEnabled:(BOOL)flag > > > { > > > - [self _setBoolValue: flag forKey: WebKitPluginsEnabledPreferenceKey]; > > > } > > > > This seems like a step beyond deprecation. This makes it so that apps that > > enable the feature won't work. > > That is correct. WebPreferences is already deprecated. We will look at > seed feedback, but we intend to remove all our NPAPI code if this goes > smoothly. Then the title and description of this bug is very misleading. You are not just disabling plugins, you are removing support for them completely. Given I have repeatedly asked about doing this and gotten the feedback that doing this requires getting data on usage, I am surprised to see this being done without that data. What has changed?

Thank you for the feedback on the title of this patch. I intend to retitle this after I finish getting the EWS data on which tests need updating. If you have feedback on the decision process of why this is being done, please redirect that feedback to the internal thread.

Comment on attachment 425763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425763&action=review >>>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:903 >>>> } >>> >>> This seems like a step beyond deprecation. This makes it so that apps that enable the feature won't work. >> >> That is correct. WebPreferences is already deprecated. We will look at seed feedback, but we intend to remove all our NPAPI code if this goes smoothly. > > Then the title and description of this bug is very misleading. You are not just disabling plugins, you are removing support for them completely. > > Given I have repeatedly asked about doing this and gotten the feedback that doing this requires getting data on usage, I am surprised to see this being done without that data. What has changed? Sam’s question seems like a good one. Hadn’t heard that we reached the point where we can do this. Not even a linked-on-or-after check?

I looked at hundreds of important apps and none of them used NPAPI plugins. The only references to NPAPI were from old embedded QTWebKit frameworks that have the ability to load plugins, but no plugins were found. In many of the environments where it is possible that NPAPI is used for other plugins, our ability to gather metrics is also disabled. This is our way to try to remove plugin support: We will try the small changes in the bugs related to this bug and listen for feedback. If we receive feedback that these are still used, it will be straightforward to revert them. Otherwise, at some time in the future we will remove the code this disables and the associated tests. One thing that has changed recently is the end of life of Flash has been reached. The possibility of someone using Flash has prevented us from doing this in the past, but that reason is now gone.

> Not even a linked-on-or-after check? To address this question specifically: A linked-on-or-after check is a good way to prevent new uses of legacy technology while giving old uses time to update and age out. But our belief is that old uses have already aged out. So, we're discussing ways to find out for sure.

<rdar://problem/76850750>

https://trac.webkit.org/changeset/275911/webkit https://trac.webkit.org/changeset/276720/webkit https://trac.webkit.org/changeset/275910/webkit