Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make all plugin-related APIs no-ops #6296

Merged
merged 2 commits into from
Feb 8, 2021
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Jan 15, 2021

Part of #6003. /cc @mfreed7 @annevk

(See WHATWG Working Mode: Changes for more details.)


/index.html ( diff )
/obsolete.html ( diff )
/system-state.html ( diff )

@domenic domenic added the removal/deprecation Removing or deprecating a feature label Jan 15, 2021
@domenic domenic mentioned this pull request Jan 15, 2021
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice. I hope we can remove some of these interfaces down the line, but this is a great start.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a cleanup bug against Firefox would be nice. Will you also file a follow-up issue on Plugin and MimeType?

@annevk
Copy link
Member

annevk commented Jan 20, 2021

It might be worth waiting with landing this until Chrome is a bit further along, just to be sure.

@domenic
Copy link
Member Author

domenic commented Jan 22, 2021

@mfreed7 did your change to Chrome come with web platform tests?

@mfreed7
Copy link
Contributor

mfreed7 commented Jan 26, 2021

@mfreed7 did your change to Chrome come with web platform tests?

@domenic no, sorry, I didn't include any WPTs for this. I did make a layout test that could easily be converted to a WPT. Should I do that? I'm still in "monitoring" mode for this change, with some anticipation of PDF-related problems, due to this comment. But I'm happy to move that test over to WPT if you think it would be valuable now.

@domenic
Copy link
Member Author

domenic commented Jan 26, 2021

Yeah, converting that test would be ideal!

@domenic
Copy link
Member Author

domenic commented Feb 5, 2021

@mfreed7 ping on converting that layout test to a WPT?

@mfreed7
Copy link
Contributor

mfreed7 commented Feb 8, 2021

@mfreed7 ping on converting that layout test to a WPT?

Yep, thanks for the ping. I was waiting to see how back the fallout was for the original change, but it seems (??) like it might be ok. I'll convert this test to WPT today.

@domenic domenic merged commit d4f07b8 into main Feb 8, 2021
@domenic domenic deleted the navigator-plugins-mimetypes branch February 8, 2021 20:06
@saschanaz
Copy link
Member

saschanaz commented Feb 9, 2021

This removes indexed property getters and thus removes @@iterator. As it could cause webcompat issue, probably need to revive the getters?

Or maybe okay when we want to completely remove the interfaces. #6317

@domenic
Copy link
Member Author

domenic commented Feb 9, 2021

How could it cause web compat issues?

@saschanaz
Copy link
Member

saschanaz commented Feb 9, 2021

for (const plugin of navigator.plugins) {} may fail if anyone is doing it.

@domenic
Copy link
Member Author

domenic commented Feb 9, 2021

I'm not sure how worried we should be about that...

@saschanaz
Copy link
Member

@annevk said we probably want to keep it for now, but yeah, no data to prove whether it will be a real issue.

@annevk
Copy link
Member

annevk commented Feb 9, 2021

If Chrome is removing them I'd be happy for Firefox to follow, to be clear. But it does mean behavior changes for us, contrary to OP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
removal/deprecation Removing or deprecating a feature
Development

Successfully merging this pull request may close these issues.

5 participants