1047191 - page-mod, "include" syntax is a pain in the rear.

Status: RESOLVED WONTFIX

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

Description

[g.litenstein]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

Trying to set-up a PageMod with user-input to determine the pages where it should actually attach to. I hit a bit of a wall with Simple-prefs only being able to store strings but before going further into circumventing that, I began toying around with what I was giving to include.



Actual results:

Despite the documentation saying that it is "A list of match pattern strings. These define the documents to which the page-mod applies."

And also, "You can specify a set of URLs using a regular expression. The pattern must match the entire URL, not just a subset, and has global, ignoreCase, and multiline disabled.
...
To specify multiple patterns, pass an array of match patterns"

It fails with one of two errors:

console.error: 
  Message: Error: When not using *.example.org wildcard, the string supplied is expected to be either an exact URL to match or a URL prefix. The provided string ('[*]') is unlikely to match any pages.

Or

console.error: 
  Message: RequirementError: The `include` option must always contain atleast one rule as a string, regular expression, or an array of strings and regular expressions.

If I use JSON.stringify on the "include" property, after a succesful setup, I see it as an associative array, but if I try to pass it either an array or an associative array formed in the same way as the one it appears to construct on its own, it breaks.
Furthermore, since it throws an _error_ (instead of say, a warning), script execution is stopped.


Expected results:

It should either actually accept lists/arrays/associative arrays of patterns or at the very least not throw an error when it arbitrarily doesn't like the supplied pattern.

Comment 1

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

Can you provide your example code that's failing?

Comment 2

[g.litenstein]

Hmm not really, I already managed to fix it through adding a more robust check on what the user inputs as a pattern. Nonetheless, doing so gave me a better understanding of what I think is broken.

page-mod does take an array for its include property. However... having even a single "invalid" element (or even something that it thinks "will not match anything, up to and including 'google.com', i.e. without either http:// or *., or even http://www.google.com/* [wildcard at end of url]) inside the array will make it throw with an error; no matter if all other items are valid.

What I was doing before was trying to load the "include" value from a string-type preference and it basically resulted in an error whenever I tried to change it (because in the process of typing, any single letter would be considered an error until I was finished). Now I'm just opening a secondary screen with a multi-line text-input and that way the user can enter several urls at once to then be stored as JSON inside my string, but not before running a series of regex and checks on each line to make sure there's nothing for page-mod to choke on.

IMO the behavior should be changed at least so that an invalid item does not make the script throw if the rest of the array has valid entries.

Comment 3

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

Irakli, any opinions on this?

Comment 4

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

I disagree as in that case author may never find out that one of the provided options was invalid and there for ignored. In other words we do prefer early errors over swallowing errors. That being said it seems that we could improve our error messages to state specific pattern that was invalid so users will have easier time tracking down errors.

Comment 5

[g.litenstein]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #4)
> I disagree as in that case author may never find out that one of the
> provided options was invalid and there for ignored. In other words we do
> prefer early errors over swallowing errors. That being said it seems that we
> could improve our error messages to state specific pattern that was invalid
> so users will have easier time tracking down errors.

I see your point, but wouldn't a warning (as opposed to an error) work just as well in the scenario I'm proposing? that is, an array with both valid and invalid entries? If none of them are valid, I think it makes sense to have an error.

Comment 6

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

(In reply to g.litenstein from comment #5)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #4)
> > I disagree as in that case author may never find out that one of the
> > provided options was invalid and there for ignored. In other words we do
> > prefer early errors over swallowing errors. That being said it seems that we
> > could improve our error messages to state specific pattern that was invalid
> > so users will have easier time tracking down errors.
> 
> I see your point, but wouldn't a warning (as opposed to an error) work just
> as well in the scenario I'm proposing? that is, an array with both valid and
> invalid entries? If none of them are valid, I think it makes sense to have
> an error.

I am afraid I disagree that warning is a better option (as those can be missed). Errors are ok, what's not ok is that it's unclear what causes that error. I have submitted bug 1070917 to resolve later issue.