-
Notifications
You must be signed in to change notification settings - Fork 77
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
Can't use set-constant.js multiple times for same parent property #156
Comments
If you test with Polish Filter, then you must also test previous version without that long filter |
I cannot reproduce with or without POL filter list. |
Then it's a filterlist issue, not a uBO issue. Investigate the cause on your end by disabling all other extensions and reseting to default settings.
The only way I can reproduce the GDPR msg is if I remove the filters you suggested in the STR and refresh the page, so not geolocked either. |
@uBlock-user I tested with default settings and disabled other extensions. If I didn't do that, then I wouldn't check it 😄 But strange is that, I tested again and now it works correct. |
He needs to make the case that the scriptlet is not executing, not that the page behave one way or another. Scriptlets are subject to timing issues, moreso depending on one's environment. For example, the more extensions installed which inject content scripts, the more chance uBO's scriptlets will be injected later, etc. In any case, the important point here is to provide evidence |
First screenshot with It's not important to me, so if you can't reproduce it then this issue can be closed. |
How about:
Seems to work on my side. |
Thank you for your answer, I checked it once again and now this rule - |
One more question, I disabled Polish filter list and added these two rules: |
I see what the problem is. |
Make sure the parser is safely compatible with old resources format -- for those users still using custom resources (via `userResourcesLocation`). Prepare code for future fix to <uBlockOrigin/uBlock-issues#156>: This commit introduces a new private Map() object, `uBOSafe`, accessible by all injected scriptlets. This private safe can be used to store data which can be shared with different scriptlets. The idea is for scriptlets to use that safe to graciously deal with the need to install multiple listeners for the same property.
Since uBlockOrigin/uBlock-issues#156 won't be fixed in next release, no need to ship with code which will be unused, and anyways only once the fix is worked on will it be clear exactly what needs to be used by scriptlets to deal harmoniously with property listener collisions.
This comment has been minimized.
This comment has been minimized.
Related issues: - uBlockOrigin/uBlock-issues#156 - uBlockOrigin/uBlock-issues#1162 Take into account that a trapped property may have been already trapped, and if so honour previous trapper getter/setter.
Related feedback: - uBlockOrigin/uBlock-issues#156 (comment) Additionally, while investigating the issue I removed code which is no longer needed since content scripts are now injected in `about:blank` frames since 1.29.0.
Actually, it just occurred to me that I can simply prevent the targeted property itself to not be configurable while allowing the parent properties leading to the target property to be configurable so as to allow trapping multiple times the same chain of properties (which is what the issue here is about). |
Related discussion: - uBlockOrigin/uBlock-issues#156 (comment) Related commit: - 6e010ec
Fixed according to AdguardTeam/AdguardFilters#65244 (comment) |
@gorhill there seems to be one more corner case when one list redefines as Case is AdguardTeam/AdguardFilters#65519 Add to My filters:
Open: cc: @krystian3w
I don't understand - it works with |
To fix this would take a lot of non-trivial code, not sure it's worth it if this can be reasonably be fixed in filter lists. Maybe that non-trivial code will occur in some future but for now it's far off. Alright, let me investigate first rather than assume I know exactly what is happening. |
And yes, after investigation, I confirm what I said above. |
@addisr Thanks for feedback. I can reproduce, and I have a fix. Now I wonder if this warrants an emergency fix. |
Related feedback: - uBlockOrigin/uBlock-issues#156 (comment) When the client code assigned a variable to itself, this would cause the scriptlet to try to re-trap already trapped properties.
CNBC is now broken - https://www.reddit.com/r/uBlockOrigin/comments/jeei8n/ublockfilters_is_breaking_cnbc/ |
Isn't gorhill/uBlock@2b262b2 actually fixing what I said above in #156 (comment) ? In tree: gorhill/uBlock@5de0ce9 |
This comment has been minimized.
This comment has been minimized.
I expect this will prevent the throwing at the console, through it won't cause the second filter to work. The more serious issue though is that it was breaking pages doing something like |
Related issues: - uBlockOrigin/uBlock-issues#156 - uBlockOrigin/uBlock-issues#1162 Take into account that a trapped property may have been already trapped, and if so honour previous trapper getter/setter.
Related discussion: - uBlockOrigin/uBlock-issues#156 (comment) Related commit: - gorhill/uBlock@6e010ec
Related feedback: - uBlockOrigin/uBlock-issues#156 (comment) When the client code assigned a variable to itself, this would cause the scriptlet to try to re-trap already trapped properties.
Related feedback: - uBlockOrigin/uBlock-issues#156 (comment) Additionally, while investigating the issue I removed code which is no longer needed since content scripts are now injected in `about:blank` frames since 1.29.0.
Related discussion: - uBlockOrigin/uBlock-issues#156 (comment) Related commit: - gorhill@6e010ec
Related feedback: - uBlockOrigin/uBlock-issues#156 (comment) When the client code assigned a variable to itself, this would cause the scriptlet to try to re-trap already trapped properties.
Prerequisites
Description
Set-constant.js doesn't work for subdomains.
A specific URL where the issue occurs
[A specific URL is MANDATORY for issue happening on a web page, even if it happens "everywhere"]
https://wiadomosci.wp.pl/
Steps to Reproduce
For example, add this:
Then go to wiadomosci.wp.pl and you should not see GDPR message with blur, but it's here. But if you add
wiadomosci.wp.pl##script:inject(set-constant.js, WP.crux.set, noopFunc)
, then everything is ok.Your environment
The text was updated successfully, but these errors were encountered: