660724 - Make sure page-mod can use regex "include" properties now that match-pattern supports them.

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Match-pattern was updated to support regexes, but page-mod is apparently still only allowing the other match-patterns:
>> let include = options.include;
>>     let rules = this.include = Rules();
>>     rules.on('add', this._onRuleAdd = this._onRuleAdd.bind(this));
>>     rules.on('remove', this._onRuleRemove =
>> this._onRuleRemove.bind(this));
>> 
>>     // Validate 'include'
>>     if (typeof(include) == "object" && !Array.isArray(include))
>>       throw new Error(ERR_INCLUDE);
>>     if (typeof(include) == "number" || typeof(include) == "undefined")
>>       throw new Error(ERR_INCLUDE);
>> 
>>     if (Array.isArray(include))
>>       rules.add.apply(null, include);
>>     else
>>       rules.add(include); 

Shouldn't match-pattern be more than capable of validating/enforcing its own rules?

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Perhaps we wanted page-mod's error messages to be more page-mod-specific?

In any case, yes, the page-mod constructor should accept regex patterns, whether it enforces its own parameters or relies on match-pattern to do so for it.

Irakli: any chance you can make this happen?  Keep in mind that the smallest, lowest-risk patch to do this is the best one, even if in the long-term there are bigger refactoring changes we may want to do here.

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/179/files

Pointer to Github pull-request

Comment 3

[Dietrich Ayala (:dietrich)]

Comment on attachment 536630 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/179/files

r=me w/ comments addressed

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Landed with merge commit:

https://github.com/mozilla/addon-sdk/commit/661e294caa97eef7fcbe64d10c6fb8503716c54f

Change commit:

https://github.com/mozilla/addon-sdk/commit/ab88a3dae7b5ae37eecca052822fd9a73291a5d7


Note: this change fixes the problem, but "pattern is undefined" is vague, and it'll be hard for addon developers who encounter the error to figure out that it's about the include option. Which is probably why PageMod has custom validation in the first place.

Anyway, I'm ok with merging this to fix the immediate problem, but we need to make this error message much better.