639781 - pageMod/MatchPattern with "*" doesn't match local files and files from data folder

Status: RESOLVED FIXED

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

Description

[Alexandre Poirot [:ochameau]]

By default, when you use "*" as MatchPattern, you won't match local file:// urls.
And this is the second time a developer don't understood why his pageMod is not working either with local html files (file://), nor files from his data url (resource://).
The solution is to use : ["*", "file://*", resource://*"] during manual tests, or for unittest.

We may either include them when using "*", 
or add visible notes about this behavior in documentation.

Comment 1

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

This was intentional, and we should definitely document it.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Improve match-pattern doc about non-remote files.

I supposed that I can affect you reviews about documentation, but do not hesitate to tell me if I'm wrong.

Comment 3

[Will Bamberg [:wbamberg]]

Comment on attachment 526014 [details] [diff] [review]
Improve match-pattern doc about non-remote files.

> I supposed that I can affect you reviews about documentation, but do not
> hesitate to tell me if I'm wrong.

Absolutely. Thanks for asking!

>diff --git a/packages/api-utils/docs/match-pattern.md b/packages/api-utils/docs/match-pattern.md
>index 5b3fe7c..4a991ca 100644
>--- a/packages/api-utils/docs/match-pattern.md
>+++ b/packages/api-utils/docs/match-pattern.md
>@@ -8,7 +8,8 @@ There are four kinds of patterns.  The first three use an asterisk as a
> glob-style wildcard.  Note that these are not regular expressions.
> 
> 1.   **A single asterisk** matches any URL with an `http`, `https`, or `ftp`
>-     scheme.
>+     scheme. It won't match any non-remote documents like local files or
>+     addon files from data folder. (see item 5.)

I think it's worth being specific that it's the scheme not the 'localness'
that makes a difference (for instance, it *would* match a localhost URL using
HTTP (I think)). Perhaps something like: 

1.   **A single asterisk** matches any URL with an `http`, `https`, or `ftp`
     scheme. Note that it won't match URLs containing any other schemes, such
     as `file` URLs: to match these you need to specify the scheme explicitly
     (see item 5.)

>      *Example:*<br>
>      &nbsp;&nbsp;&nbsp;&nbsp;**`*`**
>@@ -48,7 +49,17 @@ glob-style wildcard.  Note that these are not regular expressions.
>      *Example matching URLs:*<br>
>      &nbsp;&nbsp;&nbsp;&nbsp;`http://example.com/`
> 

Is it worth having example non-matching URLs as well?

    *Example non-matching URLs:*<br>
    &nbsp;&nbsp;&nbsp;&nbsp;`file://example.com/`

Maybe this is overkill?

>+5.  **A scheme with an asterisk** matches all document using this protocol.<br>
>+    If you want to match local files, you need to specify **`file://*`**,<br>
>+    and for addon files from data folder **`resource://*`**.

Some language/grammar nits (except addon->add-on, which isn't a nit):

5.  **A scheme followed by an asterisk** matches all documents accessed using
    that scheme.<br>
    To match local files, use `file://*`.<br>
    To match files stored in your add-on's `data` directory, use
    `resource://*`.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Comments adressed

Thanks for the review, here is a new patch with your comment adressed.

I didn't add non-matching example as there is none for others examples and I didn't found any usefull valid non-matching url for file://*

Comment 5

[Will Bamberg [:wbamberg]]

Comment on attachment 526826 [details] [diff] [review]
Comments adressed

Thanks Alex!

Comment 6

[Alexandre Poirot [:ochameau]]

Landed:
https://github.com/mozilla/addon-sdk/commit/5f5abff555cc131c2fc37a5c9daeefee6b855985