-
Notifications
You must be signed in to change notification settings - Fork 56
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
Proposal: allow storing metadata in a static ruleset #466
Comments
Safari is also fine with unknown keys. Seems fine to me, though I would suggest a name like |
Suggesting |
|
@xeenon thank you, I've updated the text and changed the field name to |
What about As for "metadata", Chrome creates |
Tbh, I am fine with any name as long as it allows to store some structured data there :) |
Even if one can, it would be nice to not add unrecognized metadata to a ruleset file. That needs to be loaded and parsed at some point. |
Hi @Rob--W, let me please comment on some points from the meeting minutes
CWS review process is indeed the main reason for the proposal. On the other hand, fast track review for DNR-only changes is a sensible thing, there's a chance that it will be implemented by other stores in a similar fashion. Therefore, having a standard way to store metadata could be useful to the teams that run these stores. If performance is a big concern, additional restrictions on metadata format can be imposed, for instance store it as unstructured single string or store metadata in a separate file. Ultimately, everything is fine as long as every browser vendor supports it. |
|
I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1886608 to track this. Note: @ameshkov confirmed in person that the intended use is to |
Firefox 128 will now silently ignore unrecognized keys in static rules and accept rules whose recognized keys have valid values. This was implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=1886608 This means that rules such as |
IMHO, a global consensus would be beneficial to "silently ignore unrecognized keys" in any settings JSON and/or similar objects, which would eliminate the need to request such policy individually in each area. |
@erosman I agree. That has been Safari's policy. |
The consensus is to avoid hard errors when an unrecognized manifest key is found: #14 Chrome and Firefox do currently print warnings in the developers UI when an unrecognized property is found, to ease development. The intentional decision to not trigger errors nor warnings for DNR enables the use of unrecognized properties at the expense of losing the ability to detect misspelled property names. |
Problem
The proposal is tightly linked to the changes announced by the Chrome WebStore team, i.e. allowing "fast track" review to extensions in the case when only static rulesets.
In the case of content blocking extensions, static rulesets is a result of a conversion from traditional ad blocking rules to the DNR syntax and in order to make debugging rules possible we need to be able to show what the source rule was. In the current AdGuard prototype extension we achieve this by packing "source maps" alongside the static rulesets.
However, updating static rulesets means updating the source maps, and this will disallow "fast track" for the extension.
Proposal
The solution would be to store the metadata within the static ruleset file. This is actually even easier to understand than the current approach that we're using now.
We checked Chrome and FF and at the moment there's no limitation on "unknown" properties used in the JSON file in Chrome, in Firefox it silently ignores rules with unknown fields. So with Chrome we can already pack metadata into the static ruleset file and be eligible for "fast track". The problem is that this is unreliable, in the future there might be new fields that will intersect with the ones that we're using, or simply the rulesets parser behavior can be changed.
So I'd propose to mention in the documentation that there's a reserved property name (
metadata
for instance) that can be used for storing additional metadata, whichever the developer wants to store there.Example:
Another example (a bit more complicated case, several source rules -> one DNR rule):
The text was updated successfully, but these errors were encountered: