757802 - context-menu.URLContext ctor should accept RegExp in addition to strings

Status: RESOLVED FIXED

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

Description

[Clément MATHIEU]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0 Iceweasel/12.0
Build ID: 20120517164517

Steps to reproduce:

I tried to pass a RegExp to URLContext ctor


Actual results:

An exception was raised due to type checking in the ctor


Expected results:

URLContext allow the user to pass MatchPattern strings and use MatchPatterns internally. MatchPatterns can be constructed from both MatchPattern string and regular expressions. 

In the current implementation, URLContext artificially limit the usage of regular expressions in a context-menu. Using regular expressions can be useful to enable an Item only for a specific page. I propose to relax the type checking performed by the ctor to also allow regular expressions.

Comment 1

[Clément MATHIEU]

Attachment: patch

Comment 2

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

Comment on attachment 626399 [details] [diff] [review]
patch

Review of attachment 626399 [details] [diff] [review]:
-----------------------------------------------------------------

I think right way to go about this would be to improve match-pattern module itself and let it handle with validations.

::: packages/addon-kit/lib/context-menu.js
@@ +500,4 @@
>    let opts = apiUtils.validateOptions({ patterns: patterns }, {
>      patterns: {
>        map: function (v) apiUtils.getTypeOf(v) === "array" ? v : [v],
> +      ok: function (v) v.every(function (p) typeof(p) === "string" || p.constructor.name === "RegExp"),

p may not have a constructor so it's error prone. You should just use `isRegExp` from "api-utils/type" module.

Comment 3

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

Clément please let me know if you plan to update a patch.

Comment 4

[Dave Townsend [:mossop]]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #2)
> Comment on attachment 626399 [details] [diff] [review]
> patch
> 
> Review of attachment 626399 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think right way to go about this would be to improve match-pattern module
> itself and let it handle with validations.

This happened as a part of bug 627386 so all we need to do is fix the validation bits. I have this fixed in my work on bug 788324

Comment 5

[Dave Townsend [:mossop]]

Fixed by bug 788324