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

PDK5 sites: Stay with the whitelist approach or build something that detects CDN loads? #43

Closed
miketaylr opened this issue Jan 24, 2019 · 3 comments

Comments

@miketaylr
Copy link
Collaborator

commented Jan 24, 2019

@wisniewskit can you send in your POC as a PR, or link it here?

It would be good to try to find some other sites using PDK5 (perhaps with HTTPArchive or similar?), and do testing on say 5 of them. If it seems OK, let's go the CDN route.

@wisniewskit

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2019

Sure, here's a link to the XPI of my POC: https://thomas.tanrei.ca/moz/pdk5fix.xpi

All that's needed to add to its list of CDNs is to find the URL variant that accesses the tdPdk.js script, and wildcard it as seen in the background.js. It might be worth testing PDK4 and lower too, if we ever find a site still using it (a UA-spoof might work fine for lower versions too).

@miketaylr

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 25, 2019

I used BigQuery + HTTP Archive to find sites in their latest million or so pages mobile run:

https://docs.google.com/spreadsheets/d/1GTlqPMvhe5UjTL_FpvLKTziLg_Qosplw06OgsghQ2z4/edit#gid=1582508588

Just 47 sites (matching against pdk[.-](player|theplatform).+5, should be enough for us to test and have some confidence.

@wisniewskit

This comment has been minimized.

Copy link
Collaborator

commented May 14, 2019

It turns out that simply spoofing the UA may be a bit much; Cooking Channel TV's site ran into long delays loading the page, for instance. As such I've decided to just edit the incoming scripts to apply the fix I suggested in https://webcompat.com/issues/8410 (basically, changing the string VideoContextChromeAndroid to VideoContextAndroid in the relevant script).

This approach seems to be working fine on at least these sites (of course, after turning off any existing UA overrides we have for these sites in about:compat):

In addition, it works fine on a version of PDK (5.9.6.536014) which doesn't need the fix and already works on Fennec (https://webcompat.com/issues/2351 is working now). It also avoids doing any UA spoofing, which is a likely win for metrics.

wisniewskit added a commit that referenced this issue May 29, 2019

Merge pull request #52 from mozilla/issue43
Roll-up of about:compat QA fixes and the PDK5 intervention. Fixes #43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.