Rename matchDomainFeatureSetting to matchConditionalFeatureSetting

Additionally ensure all checks match

Author: jonathanKingston

injected/src/config-feature.js

@@ -42,16 +42,17 @@ export default class ConfigFeature {
     }
 
     /**
-     * Given a config key, interpret the value as a list of domain overrides, and return the elements that match the current page
-     * Consider using patchSettings instead as per `getFeatureSetting`.
+     * Given a config key, interpret the value as a list of conditionals objects, and return the elements that match the current page
+     * Consider in your feature using patchSettings instead as per `getFeatureSetting`.
      * @param {string} featureKeyName
      * @return {any[]}
      * @protected
      */
-    matchDomainFeatureSetting(featureKeyName) {
+    matchConditionalFeatureSetting(featureKeyName) {
         const domains = this._getFeatureSettings()?.[featureKeyName] || [];
         return domains.filter((rule) => {
-            return this.matchConditionalChanges(rule);
+            // Support shorthand for domain matching for backwards compatibility
+            return this.matchConditionalChanges(rule.condition || { domain: rule.domain });
         });
     }
 
@@ -69,16 +70,25 @@ export default class ConfigFeature {
      * @returns {boolean}
      */
     matchConditionalChanges(conditionBlock) {
-        // Check domain condition
-        if (conditionBlock.domain && !this._matchDomainConditional(conditionBlock)) {
-            return false;
-        }
-
-        // Check URL pattern condition
-        if (conditionBlock.urlPattern) {
-            const pattern = new URLPattern(conditionBlock.urlPattern);
-            if (!this.args?.site.url || !pattern.test(this.args?.site.url)) {
-                return false;
+        for (const key in conditionBlock) {
+            switch (key) {
+                case 'domain': {
+                    if (!this._matchDomainConditional(conditionBlock)) {
+                        return false;
+                    }
+                    break;
+                }
+                case 'urlPattern': {
+                    const pattern = new URLPattern(conditionBlock.urlPattern);
+                    if (!this.args?.site.url || !pattern.test(this.args?.site.url)) {
+                        return false;
+                    }
+                    break;
+                }
+                // For unknown keys we should assume the condition is not met
+                default: {
+                    return false;
+                }
             }
         }
 
@@ -172,14 +182,12 @@ export default class ConfigFeature {
             throw new Error(`${featureKeyName} is a reserved feature setting key name`);
         }
         // We only support one of these keys at a time, where conditionalChanges takes precedence
-        // TODO should we rename these?
-        // TODO should we only support conditionalChanges to support other types of settings?
         let conditionalMatches = [];
         // Presence check using result to avoid the [] default response
         if (result?.conditionalChanges) {
-            conditionalMatches = this.matchDomainFeatureSetting('conditionalChanges');
+            conditionalMatches = this.matchConditionalFeatureSetting('conditionalChanges');
         } else {
-            conditionalMatches = this.matchDomainFeatureSetting('domains');
+            conditionalMatches = this.matchConditionalFeatureSetting('domains');
         }
         for (const match of conditionalMatches) {
             if (match.patchSettings === undefined) {

injected/src/features/element-hiding.js

@@ -320,11 +320,11 @@ export default class ElementHiding extends ContentFeature {
 
         // determine whether strict hide rules should be injected as a style tag
         if (shouldInjectStyleTag) {
-            shouldInjectStyleTag = this.matchDomainFeatureSetting('styleTagExceptions').length === 0;
+            shouldInjectStyleTag = this.matchConditionalFeatureSetting('styleTagExceptions').length === 0;
         }
 
         // collect all matching rules for domain
-        const activeDomainRules = this.matchDomainFeatureSetting('domains').flatMap((item) => item.rules);
+        const activeDomainRules = this.matchConditionalFeatureSetting('domains').flatMap((item) => item.rules);
 
         const overrideRules = activeDomainRules.filter((rule) => {
             return rule.type === 'override';

injected/src/features/navigator-interface.js

@@ -4,7 +4,7 @@ import { createPageWorldBridge } from './message-bridge/create-page-world-bridge
 
 export default class NavigatorInterface extends ContentFeature {
     load(args) {
-        if (this.matchDomainFeatureSetting('privilegedDomains').length) {
+        if (this.matchConditionalFeatureSetting('privilegedDomains').length) {
             this.injectNavigatorInterface(args);
         }
     }

injected/unit-test/content-feature.js

@@ -64,6 +64,7 @@ describe('ContentFeature class', () => {
                 expect(this.getFeatureSetting('pathTestPlaceholder')).toBe('place');
                 expect(this.getFeatureSetting('domainWildcard')).toBe('wildwest');
                 expect(this.getFeatureSetting('domainWildcardNope')).toBe('nope');
+                expect(this.getFeatureSetting('invalidCheck')).toBe('nope');
                 didRun = true;
             }
         }
@@ -86,6 +87,7 @@ describe('ContentFeature class', () => {
                     pathTestPlaceholder: 'nope',
                     domainWildcard: 'nope',
                     domainWildcardNope: 'nope',
+                    invalidCheck: 'nope',
                     conditionalChanges: [
                         {
                             domain: 'example.com',
@@ -103,38 +105,59 @@ describe('ContentFeature class', () => {
                             patchSettings: [{ op: 'replace', path: '/arrayTest', value: 'enabledArray' }],
                         },
                         {
-                            urlPattern: {
-                                path: '/path/path/me',
+                            condition: {
+                                urlPattern: {
+                                    path: '/path/path/me',
+                                },
                             },
                             patchSettings: [{ op: 'replace', path: '/pathTest', value: 'beep' }],
                         },
                         {
-                            urlPattern: {
-                                hostname: 'beep.nope.com',
-                                path: '/path/path/me',
+                            condition: {
+                                urlPattern: {
+                                    hostname: 'beep.nope.com',
+                                    path: '/path/path/me',
+                                },
                             },
                             patchSettings: [{ op: 'replace', path: '/pathTestNotApply', value: 'yep' }],
                         },
                         {
-                            urlPattern: 'http://beep.example.com/path/path/me',
+                            condition: {
+                                urlPattern: 'http://beep.example.com/path/path/me',
+                            },
                             patchSettings: [{ op: 'replace', path: '/pathTestShort', value: 'beep' }],
                         },
                         {
-                            urlPattern: 'http://beep.example.com/*/path/me',
+                            condition: {
+                                urlPattern: 'http://beep.example.com/*/path/me',
+                            },
                             patchSettings: [{ op: 'replace', path: '/pathTestAsterix', value: 'comic' }],
                         },
                         {
-                            urlPattern: 'http://beep.example.com/:something/path/me',
+                            condition: {
+                                urlPattern: 'http://beep.example.com/:something/path/me',
+                            },
                             patchSettings: [{ op: 'replace', path: '/pathTestPlaceholder', value: 'place' }],
                         },
                         {
-                            urlPattern: 'http://beep.*.com/*/path/me',
+                            condition: {
+                                urlPattern: 'http://beep.*.com/*/path/me',
+                            },
                             patchSettings: [{ op: 'replace', path: '/domainWildcard', value: 'wildwest' }],
                         },
                         {
-                            urlPattern: 'http://nope.*.com/*/path/me',
+                            condition: {
+                                urlPattern: 'http://nope.*.com/*/path/me',
+                            },
                             patchSettings: [{ op: 'replace', path: '/domainWildcardNope', value: 'wildwest' }],
                         },
+                        {
+                            condition: {
+                                somethingInvalid: true,
+                                urlPattern: 'http://beep.example.com/*/path/me',
+                            },
+                            patchSettings: [{ op: 'replace', path: '/invalidCheck', value: 'neverhappened' }],
+                        }
                     ],
                 },
             },