Reading through the Mozilla documentation it looks like this property is considered legacy and they suggest allow="fullscreen" as the preferred attribute now. It seems that supporting the allow attribute with perhaps a sum type covering the available cases would be the better long-term fix.

@thomashoneyman that's a good idea. Do you know if allow accepts multiple values?

It seems that allow="fullscreen" and allow="payment" both use the same key but I do not see any docs about using both at the same time.

Also, should the sum type go in src/Halogen/HTML/Properties.purs?

Do not accept yet I am adding multiple value capabilities

now takes multiple values

It looks like it uses the syntax of feature policies that also can be set in HTTP requests: https://developer.mozilla.org/en-US/docs/Web/HTTP/Feature_Policy/Using_Feature_Policy

Syntax and list of features here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy

Maybe a Map would be appropriate?

It is annoying that they "allow" so many syntactic differences.

@MonoidMusician Didn't use Map but did include a key value constructor.

@MonoidMusician @thomashoneyman Is there anything else that needs to be done here?

I took a moment to walk through the documentation on MDN. I’d like to share some snippets from the documentation that describe the purpose and syntax of this policy — I wasn’t aware of this information until this moment, so I’d like to share these in case other reviewers are also unfamiliar with how this works! Then, with this knowledge in hand, I’d like to share what adjustments I think should be made to the types.

The Feature Policy page summarizes what this policy is about:

With Feature Policy, you opt-in to a set of “policies” for the browser to enforce on specific features used throughout a website. These policies restrict what APIs the site can access or modify the browser’s default behavior for certain features.

You can enforce some restrictions on APIs and features that a website can use. This can be done via an HTTP header or via the allow attribute on an <iframe>, which is what we care about in this instance. In both cases the idea and the syntax seem to be the same. Those are documented further on the Using Feature Policy page:

Feature Policy allows you to control which origins can use which features…essentially, you write a policy, which is an allowed list of origins for each feature. For every feature controlled by Feature Policy, the feature is only enabled in the current document or frame if its origin matches the allowed list of origins…if you do not specify a policy for a feature, then a default allowlist will be used. The default allowlist is specific to each feature.

A policy is described using a set of individual policy directives. A policy directive is a combination of a defined feature name, and an allowlist of origins that can use the feature. Features within a policy are separated by semicolons.

This gives more direction on what exactly allow is controlling: it lets you write a ‘feature policy’ by combining a set of individual ‘policy directives’, where each policy directive combines a feature name with an allowlist of origins that can use the feature.

Using the MDN terminology brings to mind another way to structure the types:

• allow accepts a FeaturePolicy
• A FeaturePolicy is a Set PolicyDirective (or Array for convenience’s sake)
• Each PolicyDirective combines a FeatureName with an Allowlist
• An Allowlist can be empty, which will rely on the defaults, or it can be one or more Origin

There are many ways we could define types to capture this structure. For example, a minimal version might say that allow accepts an Array PolicyDirective and each PolicyDirective is a simple tuple of a FeatureName and an Array Origin, where an empty array represents the default.

But there’s yet more detail about what constitutes an acceptable allowlist:

An allowlist is a list of origins that takes one or more of the following values, separated by spaces:

• *: The feature will be allowed in this document, and all nested browsing contexts (iframes) regardless of their origin.
• ‘self’: The feature will be allowed in this document, and in all nested browsing contexts (iframes) in the same origin.
• ‘src’: (In an iframe allow attribute only) The feature will be allowed in this iframe, as long as the document loaded into it comes from the same origin as the URL in the iframe’s src attribute.
• ‘none’: The feature is disabled in top-level and nested browsing contexts.
• <origin(s)>: The feature is allowed for specific origins (for example, https://example.com). Origins should be separated by a space.

The values * (enable for all origins) or ‘none’ (disable for all origins) may only be used alone, while ‘self’ and ‘src’ may be used with one or more origins.

So it’s not quite correct that you can just have a list of origins. Instead, you can either have:

1. The origin *
2. The origin ’none’
3. The origins ’src’, ’self’, and/or a list of further space-separated origins

So perhaps we would want to have a Allowlist type that captures these possibilities rather than just being a flat array of origins. After all, it’s not correct that you could do an allow list like:

[ All, Self, None, Origin "https://example.com" ]

Turning now to what features might make up a FeatureName, I took a look at the full list of supported feature directives, and it seems like we ought to at least support directives with Chrome, Edge, Firefox, Opera, or Safari compatibility.

In the current type only two of these directives are represented (”fullscreen” and ”payment”) and the rest would have to be written manually the Allow String or AllowMultiple (Array Allow) or AllowOrigin String Origin constructors, but I think we should support all the directives with at least one major non-IE browser supporting the feature.

Whew! Sorry for such a lengthy dive into the documentation, but I wanted to get this information out into this PR so we can all use it as a basis for further discussion. With all of this in mind, I think that we should adjust these types a little bit to accommodate the MDN specification.

Here’s what I propose (but I’m open to other suggestions):

First, allow should be restricted to accept policy directives. The policy directive type should either be:

1. An Array PolicyDirective or a Set PolicyDirective, where a PolicyDirective is a pairing of a FeatureName with an optional Allowlist.
2. A Map FeatureName (Maybe Allowlist) — this is what @MonoidMusician suggested, and because keys have to be unique this automatically gives us our set behavior that’s called for in the spec (this seems like the best option to me).

Second, FeatureName should be a type which covers all supported feature directives that a major non-IE browser can leverage.

Third, the Allowlist type should properly represent that it can be ‘*’, ‘none’ or ‘self’, ‘src’, and/or a list of strings representing other origins.

Finally, we should have a string renderer which follows these rules:

1. A policy directive is rendered as a semicolon-separated list of feature names paired with their allowlists
2. An allowlist is rendered as a space-separated list of origins

As a bonus (which I’m happy to add on to your PR as a follow-up), it would be really nice to have a test that makes sure that the formatting is correct for various cases:

-- fromFoldable [ Tuple Fullscreen Nothing ]
"fullscreen"

-- fromFoldable [ Tuple Fullscreen (Just [ SrcOrigin ]) ]
"fullscreen 'src'"

-- fromFoldable [ Tuple Geolocation (Just [ Origin "https://example.com", "https://google-developers.appspot.com" ]) ]
"geolocation https://example.com https://google-developers.appspot.com"

-- fromFoldable [ Tuple Camera (Just NoOrigin), Tuple Microphone (Just NoOrigin) ]
"camera 'none'; microphone 'none'"

Finally, it’s worth noting this little warning in the MDN documentation:

The Feature-Policy header has now been renamed to Permissions-Policy in the spec, and this article will eventually be updated to reflect that change.

So perhaps any reference to ‘Feature’ in this review should really be ‘Permission’

@BebeSparkelSparkel I understand this bumps up the work necessary to get this PR merged, and I'm sorry to be requesting that additional work. On the other hand, doing so would mean you're really adding the ability to support permission policies to Halogen, which is a pretty big (and welcome) addition!

Lot of things going on right now. I'll try read this soon