fix: youtube & iframe util from packages/ui to main source #1884
Conversation
smalluban
force-pushed
the
fix-youtube-and-iframe-util
branch
3 times, most recently
from
af6e22e to
7e22997
Compare
smalluban
force-pushed
the
fix-youtube-and-iframe-util
branch
from
7e22997 to
23ad974
Compare
smalluban
force-pushed
the
fix-youtube-and-iframe-util
branch
from
96c9467 to
7eafd79
Compare
| const TEST_COOKIE_NAME = `ghostery:opera:cookie:test:${Date.now()}`; | ||
|
|
||
| let isSupported = undefined; | ||
| export async function isSerpSupported() { |
There was a problem hiding this comment.
From what I see, this function may be called concurrently. If so, could setting the cookies then conflict?
There was a problem hiding this comment.
I think setting the cookie just overwrites the values when called multiple times. I carefully tested the feature on Opera, and I haven't found issues.
| @@ -57,13 +57,14 @@ if (argv.target.startsWith('safari')) { | |||
| argv.target = 'safari'; | |||
| } | |||
|
|
|||
| // Add flags to manifest | |||
| if (argv.debug) manifest.debug = true; | |||
| if (argv.staging) manifest.staging = true; | |||
There was a problem hiding this comment.
was wondering why you "invent" the stating mode, but from the other changes it looks like it was implicitly there for some subsystems.
We have to decide how those modes should work.
Historically:
- debug - was setting all backends to staging and was enabling some additional logging
- staging - was recently introduced to download adblocker engines from the staging environment
It looks like we have 3 separate aspects to deal with:
- debugging of the addon (logs, some settings like opera serp, scritplets debugging)
- adblocker engines environment
- backends (staging vs production)
There was a problem hiding this comment.
The staging mode usually does not work. They it yourself. The engines are not updated there (I suppose we have to trigger it manually).
Firstly, there was only a debug mode, but with that issue I had to add separate flag for loading engines from staging.
There was a problem hiding this comment.
ah, so you mean even if we downloaded staging engine pre-build, they would update to production engine in the runtime anyways?
I'm fine to keep the staging mode. Shall we then update all endpoints configuration to use stagingMode instead of debugMode?
There was a problem hiding this comment.
npm start -- --staging fails... as the staging CDN does not have the proper version of the engines..
For me --staging should only set the engines' endpoints - as this must be used only when we know that we pushed something to the staging CDN for engines and only engines.
Error: Engine "dnr-ads" for "669" engine version not found
at file:///Users/dominik/Workspace/ghostery-extension/extension-manifest-v3/scripts/download-engines.js:89:11
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)There was a problem hiding this comment.
the problem occurs as we don't create staging engines on every adblocker release - perhaps we should. When this happen please reach out so we can schedule a CI build.
Co-authored-by: Philipp Claßen <philipp@ghostery.com>
youtubefrompackages/uito the main sourceiframefeaturepages/notificationsto group all notifications in one pathopera-serputildebugModefor easier testing andstagingModefor allowing engines to update from staging CDNI tested the Youtube Wall notification (Chrome, Firefox, Safari) and Opera Serp (Opera).