Closed AdamWr closed 2 years ago
It is because this scriplet uses getters and setters (in Object.defineProperty
) instead of just value
here:
It is generally unreasonable. It modifies objects a lot.
I have this result:
Using just value is not possible because, at the moment when the scriptlet is executed, there may be no test
object at all. We need to detect when it's created and only then we can define the property.
Solving this requires having a "context" object that can be used by scriptlets to "communicate". I suppose it'd be better to do this after the "new approach" task: https://github.com/AdguardTeam/Scriptlets/issues/83
Using get
/set
and value
doesn't differ in this. You still need to detect creating of object even with get
/set
.
A brief comment on this: set-constant now creates configurable property, and in this case, it is possible to "chain" property calls. This does solve the immediate issue but makes it possible for page scripts to override this property on its own. We should keep an eye on what websites do with this.
I'm not sure if it's related to this problem, but it seems that using delete
operator causes that set-constant
and abort-on-property-read/write
scriptlets with chained property do not work.
Example code:
var test = {};
delete window.test;
var test = {
number: 999,
boolean: true
};
Test page - https://adamwr.github.io/test-page/delete.html
Rule:
adamwr.github.io#%#//scriptlet("set-constant", "test.number", "0")
or
adamwr.github.io#%#//scriptlet("abort-on-property-write", "test.number")
There is a console.log
function which prints test.number
, so for set-constant
scriptlet it should shows 0
in dev console, and for abort-on-property-write
it should shows undefined
, but it seems that scriptlets do not work.
It bothers me that this task takes that much time to solve, there are already quite a lot of places in the filters where we need this behavior.
If it does not fit into the current set-constant
implementation, we might want to consider adding a different scriptlet or adding a new parameter to set-constant
that will change its logic.
@slavaleleka @maximtop guys, what do you think about that?
Values can be used instead of setters and getters if we do not need any checks when someone tries to change this "constant".
From @ameshkov: (19 Aug 2020)
A brief comment on this: set-constant now creates configurable property, and in this case, it is possible to "chain" property calls. This does solve the immediate issue but makes it possible for page scripts to override this property on its own. We should keep an eye on what websites do with this.
:arrow_right_hook: The bold part is a no-no for me. Too risky and problematic over time, even for the follow-up of its own commits over time … Especially for large websites that can easily afford to quickly counter. :confused:
For info/context, it would have been useful to me recently: :v:
see https://github.com/AdguardTeam/AdguardFilters/pull/102134 > part Bonus n°1 — Websites from the TF1 galaxy …
.
For example, if there is a script like this:
and we want to set
test.number
to0
andtest.boolean
tofalse
using scriptlets like:#%#//scriptlet("set-constant", "test.number", "0")
#%#//scriptlet("set-constant", "test.boolean", "false")
then only one of these scriptlets work actually.Expected behavior - both scriptlets should work.
Test page - https://adamwr.github.io/test-page/index.html Rules:
adamwr.github.io#%#//scriptlet("set-constant", "test.number", "0")
adamwr.github.io#%#//scriptlet("set-constant", "test.boolean", "false")
Screenshot
![image](https://user-images.githubusercontent.com/29142494/71629799-bfa2e500-2bff-11ea-8fd6-41fb51b00d39.png)