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
Assignees

Comments

@miketaylr
Copy link

@miketaylr miketaylr 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
Copy link
Collaborator

@wisniewskit wisniewskit 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
Copy link
Author

@miketaylr miketaylr 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
Copy link
Collaborator

@wisniewskit wisniewskit 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
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
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants