document page mod regexes, including their restrictions

RESOLVED FIXED in 1.0

Status

P2
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: myk, Assigned: wbamberg)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
In bug 653495, we're going to restrict the regexes that can be used as match patterns for page mods.  We should document these restrictions in 1.0b5, even if we don't actually land the code that implements the restrictions by then, so users know what to expect in the future.
(Reporter)

Updated

7 years ago
Priority: P1 → P2
Summary: document restrictions on page mod regexes → document page mod regexes, including their restrictions
Target Milestone: 1.0b5 → 1.0
(Assignee)

Comment 1

7 years ago
Created attachment 536189 [details] [diff] [review]
Document regexes for match-pattern
Attachment #536189 - Flags: review?(warner-bugzilla)
A few thoughts:
* in all cases where the docs talk about URLs, they really mean strings that
  contain URLs, right? (and not URL instances obtained from require("url")).
  It'd probably be more confusing to mention require("url") than to leave it
  the way it is, but I thought I'd confirm.
* Exact Matches: might be good to mention "(starting with a scheme, ending
  with a slash, and containing no wildcards)". It's also not clear how a
  trailing slash would be accepted.. someone might try "http://example.com"
  and expect it to work just as well as "http://example.com/"
* "*.example.com" only matches http, https, and ftp schemes, right?. i.e. the
  "regardless of scheme" note really means regardless of which one of those
  three schemes it uses?
* (maybe have a "does *not* match URLs like this.." column, with samples in
  red, for contrast)
* "to match files stored in your add-on's data directory, use resource://*" :
  won't that also match files coming from other addons? To really restrict it
  to files coming from this one addon, I think we'd need to use
  require("self").data.url(""), and commit to allowing data.url() to be used
  with names that don't actually exist in the data/ directory (i.e. expose
  some of the URL structured used for self.data, instead of keeping it
  opaque)
* in the regexp /.*moz.*/ example, it would match http://hemozoon.com too,
  right? ('grep moz /usr/share/dict/words' comes up with the weirdest hits..
  now I want to find out what squamozygomatic means). Also
  https://mozilla.org . Also would it match a weird scheme like
  "mozscheme://host/path"? The bare example makes it slightly seem like the
  regexp is matched against just the URL's path, not against the whole URL
  (including scheme).
* for /http:\/\/moz.*/ , pointing out that it doesn't match
  https://mozilla.org might be good
* for /http.*moz.*/ , that also matches http://www.http.randommoz.com/ ,
  yeah? Not sure it's worth pointing out all these corner cases, but weird
  samples like that always jump out at me when I'm trying to understand docs
  like these.
(Assignee)

Updated

7 years ago
Attachment #536189 - Flags: review?(warner-bugzilla)
(Assignee)

Comment 3

7 years ago
Created attachment 536499 [details] [diff] [review]
Added more examples, and so on.

Thanks for the comments.

> * (maybe have a "does *not* match URLs like this.." column, with samples in
>   red, for contrast)

I thought making it red sort of unbalanced the page.

> * in the regexp /.*moz.*/ example, it would match http://hemozoon.com too,
>   right? ('grep moz /usr/share/dict/words' comes up with the weirdest hits..
>   now I want to find out what squamozygomatic means). 

:-) squamozygomatic.org is not taken!

> Also
>   https://mozilla.org . Also would it match a weird scheme like
>   "mozscheme://host/path"? The bare example makes it slightly seem like the
>   regexp is matched against just the URL's path, not against the whole URL
>   (including scheme).
> * for /http:\/\/moz.*/ , pointing out that it doesn't match
>   https://mozilla.org might be good
> * for /http.*moz.*/ , that also matches http://www.http.randommoz.com/ ,
>   yeah? Not sure it's worth pointing out all these corner cases, but weird
>   samples like that always jump out at me when I'm trying to understand docs
>   like these.

To me, as well, the corner cases are really useful. I didn't include http://www.http.randommoz.com/ because it seemed to me that it's covered by http://hemozoon.org/, while the second 'http' doesn't change the truth value of the regex but might just be confusing.

The "Examples" section now looks a little sad - it might be better to remove it, and include some code examples of each type along with the table. What do you think?
Assignee: nobody → wbamberg
Attachment #536189 - Attachment is obsolete: true
Attachment #536499 - Flags: review?(warner-bugzilla)
Comment on attachment 536499 [details] [diff] [review]
Added more examples, and so on.

Awesome.. the match-vs-not-match examples make it really clear. Double points for using "mozzarella" in a URL, triple-word-score for including hemozoon :).

r+ with two small fixes:

 1: the first sentence in the "Wildcards" section has a forward-reference to "(For other schemes like file:, see item 5)", but it's not clear what the 5 refers to. If it's to last part of the "Wildcards" section, by my count that might be item 4. Maybe just say "see below"?

 2: the /.*moz.*/ section with the first hemozoon.org reference needs an extra <br>, on my screen I see http://foo.com/mozilla and http://hemozoon.org on the same line
Attachment #536499 - Flags: review?(warner-bugzilla) → review+
(Reporter)

Comment 5

7 years ago
a=myk to land this during freeze.
(Assignee)

Comment 6

7 years ago
Landed in https://github.com/mozilla/addon-sdk/commit/627aee2758d719b55e5433c00ca38a94baa9538e.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.