As we found out within Bug 1714966 we should verify that the matching algorithm works correctly with https-first mode enabled.

In particular, does
matches: ["http://example.com/dummy"],

match the https version of it?

I'm not clear what the intention here is, my initial reaction is that it should not match to https.

(In reply to Shane Caraveo (:mixedpuppy) from comment #2)

I'm not clear what the intention here is, my initial reaction is that it should not match to https.

I guess the question we need to answer or ask ourselves. Now that we do so much auto-upgrading of http to https then those extensions wouldn't work, right? In CSP for example, we are allowing the secure equivalent of a resource to match - I think this is what we should do here as well, but I can be convinced otherwise.

What's your take?

I saw the patch before this bug, so I've commented there already: https://phabricator.services.mozilla.com/D117425#3822649

matches from extensions should match the actual URL of the document. How does the upgrade feature work?
If the upgrade feature works by redirecting http to https, then http: should not match but http: should. Extensions can already explicitly match by http / https / * (either) if they want. We don't need to "upgrade" http:-matching.

Match patterns match based on the requested URL. If we were to match http for https requests, then that may cause problems, e.g. for HTTPS Everywhere, which matches by http:-schemes and upgrades such resources.

Now that we do so much auto-upgrading of http to https then those extensions wouldn't work, right?

I think they continue working exactly as intended. As Rob mentions, they can match both if that is needed, and the risk of breakage or unexpected behavior is higher if we allow an http match to also match https. I also suspect that it is not common to match on http except in specific types of extensions.

Lets keep our matching working as defined and documented.

(In reply to Shane Caraveo (:mixedpuppy) from comment #5)

Lets keep our matching working as defined and documented.

Ok, fair enough. I am going to land that test anyway, but in that case making sure that the https-first upgrade does not match in case matches http: is used.

Putting this in the backlog for now. It seems we need to re-write that test and use a different strategy because mochitests apparently do not support what we want to test here.