1367320 - Exclude in Match patterns

Status: RESOLVED WONTFIX

(WebExtensions :: General, enhancement)

Description

[erosman]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170505181442

Steps to reproduce:

There are occasions when a negative match to exclude patterns would be greatly useful.

A prime example is documentUrlPatterns in contextMenus.create() where a method to exclude URLs and prevent contextMenus from appearing would be beneficial.

In fact, incorporating negative matches in Match patterns could even simplify other APIs (like "exclude_matches") and all APIs that use Match patterns

An idea would be to parse '!' at the start of pattern "!*://*.mozilla.org/*" 

It should be simple enough to implement and if desired, I might be able to write the extra code needed for "MatchPattern.jsm" and pass it on to be tested/implemented

Comment 1

[Andy McKay]

This has been added to the agenda for the May 30 WebExtensions Triage meeting at 10:30am PT.

Call in info: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Details_.26_How_to_Join
Agenda: https://docs.google.com/document/d/1hKKRpGFIaAaI3G_HfPX2Nk8pCchyhUIKJB9y5sIrVV4/edit

Comment 2

[Andrew Swan [:aswan]]

Re-targeting this bug to be about the specific application described in the original report and marking that as approved as discussed in the community meeting.
Match patterns are used for a number of different things, many of which are performance- and security-sensitive, so we're not inclined to add complexity to them without a very compelling reason.  erosman, if you think you have a compelling reason, please provide much more detail.

Comment 3

[erosman]

This request is NOT limited to contextMenus.create()
Changing its title would diminish its effect and usefulness and give the wrong and narrow perspective.

As mentioned, "In fact, incorporating negative matches in Match patterns could even simplify other APIs (like "exclude_matches") and all APIs that use Match patterns"

AFA overheads, a few line change to the "MatchPattern.jsm" does it all with no extra overheads and no change needed to any other existing API.
I can think of no security issues whatsoever with the inclusion of negative matches. All it does, is to return a simple true/false

Example: (whatever API)

[
  "*://*.mozilla.org/*" , // match mozilla.org
  "!*://*.bugilla.org/*" // dont match bugzilla.org
]


[
  "<all_urls>", // match all supported
  "!*://addons.mozilla.org/*" // but dont match addons.mozilla.org
]

(Simplistic example... not a real situation)


I can write an example of the changes needed just to show how simple it could be.

Example of APIs that would benefit transparently (without the need to any modification)
Content scripts
contextMenus: documentUrlPatterns, targetUrlPatterns
webRequest
etc

I don't have a complete list of all APIs that use Match patterns

Comment 4

[erosman]

The tile "Add excludeDocumentUrlPatterns option to contextMenus.create()" suggest a new API added to the existing contextMenus.create() which is not the purpose or the intention of this filing.

Comment 5

[Andrew Swan [:aswan]]

(In reply to erosman from comment #3)
> This request is NOT limited to contextMenus.create()
> Changing its title would diminish its effect and usefulness and give the
> wrong and narrow perspective.
> 
> As mentioned, "In fact, incorporating negative matches in Match patterns
> could even simplify other APIs (like "exclude_matches") and all APIs that
> use Match patterns"

This is too vague to be useful.

> AFA overheads, a few line change to the "MatchPattern.jsm" does it all with
> no extra overheads and no change needed to any other existing API.

That is irrelevant since MatchPattern.jsm is on the verge of being removed.

> I can think of no security issues whatsoever with the inclusion of negative
> matches.

That's a poor argument for adding more complexity.  The point is that match patterns are used in cases where we want it to be easy to reason about how they work and to verify that the implementation matches our expectations.  As a consequence, the implementation needs to be simple and straightforward.  You need to provide a compelling application for adding functionality to them to offset the extra code that is required to implement that functionality.

> All it does, is to return a simple true/false
> 
> Example: (whatever API)
> 
> [
>   "*://*.mozilla.org/*" , // match mozilla.org
>   "!*://*.bugilla.org/*" // dont match bugzilla.org
> ]

I don't understand, the match pattern [ "*://*/mozilla.org/*" ] already does not match bugzilla.org, what is gained by adding that clause?

> (Simplistic example... not a real situation)

If you're serious about this, please provide a concrete use case for it.

> Example of APIs that would benefit transparently (without the need to any
> modification)
> Content scripts
> contextMenus: documentUrlPatterns, targetUrlPatterns
> webRequest
> etc

You state that these APIs will benefit, please explain how.

Comment 6

[Andrew Swan [:aswan]]

Whoops, didn't mean to change the title back since the original reporter was adamant about that.  But since this bug is staying with the broad request, closing it unless/until we have specific and compelling reasons to add the requested capability.

Comment 7

[erosman]

I am astonished that I have to explain this, but here we go....

In layman's terms ..... 

Match patterns: a system to check a given URL against a set of rules to get true/false match result

Positive Pattern: Returns true if the given URL matches the set of rules, otherwise returns false
Negative Pattern: Returns false if the given URL matches the set of rules, otherwise returns true

Difference in Security implication: none
Checking if a URL matches a pattern or not, has zero security implication for the Match pattern system

Example:
rules:
[
  "*://developer.mozilla.org/*" , // match developer.mozilla.org
  "!*://developer.mozilla.org/de/*" // dont match developer.mozilla.org/de
]

Target URL: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Match_patterns
URL passed to Match patterns return true

Target URL: https://developer.mozilla.org/de/Add-ons/WebExtensions/Match_patterns
URL passed to Match patterns return false


In simple words... all developer.mozilla.org URL ... except developer.mozilla.org/de/*


rules:
[
  "<all_urls>", // match all supported
  "!*://addons.mozilla.org/*" // but dont match addons.mozilla.org
]


Target URL: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Match_patterns
URL passed to Match patterns return true

Target URL: https://addons.mozilla.org/en-US/developers/addons
URL passed to Match patterns return false


In simple words... all ... except addons.mozilla.org


Usage & Benefit:
In a fashion similar to implementation of "exclude_matches", the excluded can be incorporated in the Match patterns without the need for a separate API property ("exclude_matches")

Why needed:
Often developers need to exclude certain URLs for their purpose (it can be page URL, web request, node url etc).
The current system does not allow for such exclusion with the exception of above mentioned "exclude_matches".

In layman's term, developer would like to set a set of rules to allow action, except some URLs (ie all Google URLs expect google.jp)

There are other instances where a long list of positive matches can be replaced with a single negative match.

The current system entails allowing all and then programmatically checking URLs again in background or content script in order to exclude unwanted URLs.

AFA contextMenu (for example) that is not even possible as the menu appear before any checks can be made. However, the usage is NOT limited to contextMenu and implementation can benefit all APIs that use Match Pattern (thus the need for the title to be specific to match Pattern and not contenMenu)


I hope that helps....

Finally, this is only a suggestion for the improvement of Firefox. It does not affect me personally either way. Therefore, I am not adamant whether it should be taken up or not.

IMHO, that would improve Firefox but if you disagree, that is your prerogative.

Comment 8

[kernp25]

> Add excludeDocumentUrlPatterns option to contextMenus.create

I like this idea. Can we have this option?
And also for targetUrlPatterns?

Comment 9

[Andrew Swan [:aswan]]

I don't believe anything has changed since the original decision was made.

Comment 10

[kernp25]

I mean something like this:
browser.contextMenus.create({
  id: "my-link-item",
  title: "My Link Item",
  contexts: ["link"],
  excludeTargetUrlPatterns: ["*://example.com/*"]  
});

The above context menu item does not display on links which have the host "example.com" but does display on all other links.

Comment 11

[Andrew Swan [:aswan]]

Oh sorry, I misread your comment, please open a new bug.

Comment 12

[kernp25]

https://bugzilla.mozilla.org/show_bug.cgi?id=1385073

Comment 13

[erosman]

My suggestion would have encompassed all those conditions without a need for a separate API alteration to multiple APIs that use the match patterns

The only file that needed to change was "MatchPattern.jsm" (and I could have written the code to help out)

That meant:

- Developers could exclude in contextMenus (without any new changes to the API)
- Developers could exclude in manifest.json (without any new changes to the API)
- Developers could exclude in webRequest (without any new changes to the API)
- documentUrlPatterns
- targetUrlPatterns
- or any other API that uses match patterns

For example in contextMenus it would have looked like:

browser.contextMenus.create({
  id: "my-link-item",
  title: "My Link Item",
  contexts: ["link"],
  targetUrlPatterns: ["!*://example.com/*"]  
});


Nonetheless, Firefox (or any other browser) is in need of an exclusion method for match patterns.

Often developers need to implement a condition such as: Allow ALL, Except SOME

Adding exclude property/filter to each of the APIs that use match pattern seems a lot more work, especially if it could have been avoided.