Amphtml: Consider allowing input[type=button] and input[type=image]

Created on 16 Jan 2020  路  19Comments  路  Source: ampproject/amphtml

I'm coming across some WordPress themes that are using input[type=button] and input[type=image]. Are there specific reasons for why are not allowed in AMP?

image

https://github.com/ampproject/amphtml/blob/a81762bd82cac406c3341a02908e45ed9448c560/validator/validator-main.protoascii#L4509-L4517

I'm wanting to know if either AMP should allow them or otherwise if the WordPress AMP plugin should try to convert them to <button> and <button><amp-img></amp-img></button> respectively.

When Possible Feature Request caching

Most helpful comment

Sounds like the conclusion is:

  • Add button / file to the supported list.
  • Modify the AMP transformers to rewrite images found in <input src> using the same logic as <img src>.
  • After the above has been released, create a new validator rule for <input type=image> that also requires src, width, and `height.

It doesn't sound like there is demand for type=password, so for simplicity perhaps continue to disallow it until there is a use case.

cc @ssantosms to be aware of the image rewrite.

cc @ampproject/wg-amp4email for thoughts on which of these should apply to the email spec.

@lannka for thoughts on which of these should apply to the ads spec.

@choumx for thoughts on which of these should apply to the actions spec.

All 19 comments

/cc @ampproject/wg-security-privacy per @honeybadgerdontcare in Slack.

Per @Gregable in the Slack thread:

My memory may be completely corrupt here, but I vaguely recall something about wanting to avoid someone creating a document which looked like a Google login page to be used for Phishing.

It seems that with the dynamic elements that AMP supports these days, that this ship sailed long ago.
Therefore, if my memory is correct, then this can be relaxed in the validator.

The type=password is what makes me think of this, but I really could be wrong.

The issue with <input type="image">, AFAIR, that we can't control its loading time it jumps the layout once loaded. And at this point we can't even apply layout rules to it. Is there a discernable difference between using <input type=image src="..."> vs <input type=submit><amp-img src="..."></input>?

An input[type=image] can have a width/height, so that would prevent layout shifting.

The only thing that input[type=image] doesn't support is loading=lazy, so that would prevent deferring of the image loading.

Is there a discernable difference between using <input type=image src="..."> vs <input type=submit><amp-img src="..."></input>?

With styling it doesn't appear so.

<code>input[type=image]</code> 馃憠
<input type="image" src="https://amp.dev/static/img/favicon.png" width="50" height="50">

<button style="border: none; padding: 0; margin: 0; background: transparent; cursor: pointer; -webkit-appearance: none; -moz-appearance: none;"><amp-img src="https://amp.dev/static/img/favicon.png" width="50" height="50"></amp-img></button>
馃憟 <code>button &gt; amp-img</code>

image

So transforming input[type=image] to button > amp-img seems like a viable option if it remains invalid AMP.

Sounds like the conclusion is:

  • Add button / file to the supported list.
  • Modify the AMP transformers to rewrite images found in <input src> using the same logic as <img src>.
  • After the above has been released, create a new validator rule for <input type=image> that also requires src, width, and `height.

It doesn't sound like there is demand for type=password, so for simplicity perhaps continue to disallow it until there is a use case.

cc @ssantosms to be aware of the image rewrite.

cc @ampproject/wg-amp4email for thoughts on which of these should apply to the email spec.

@lannka for thoughts on which of these should apply to the ads spec.

@choumx for thoughts on which of these should apply to the actions spec.

  • After the above has been released, create a new validator rule for <input type=image> that also requires src, width, _and_ `height.

Yep. Exactly. As long as we rewrite the src, we can allow <input type=image> with the required width and height values.

One other FYI. In addition to not supporting loading=lazy, we also lose support for srcset (at least I didn't find any reference to <input srcset>). The AMP cache applies this automagically to images. The cache and transformers will need to simply rewrite src only. I think this is a reasonable tradeoff.

The cache transforms were filed internally at Google as b/148083520.

@Gregable, is there no srcset in <input type=image>?

I didn't test it, but quick reference checks didn't see a reference to it, such as here, here, or here.

I suppose it doesn't make me like <input type=image> anymore than I already didn't like it. But I just checked, it's not deprecated, so let's do.

Some quick thoughts from email PoV:

  • type="button" sounds OK.
  • type="image" would add another resource that can potentially leak requests and that'd need to be proxied, unless there's a strong reason, this is probably not worth it.
  • type="password" and type="file" aren't currently allowed in any capacity (as emails only support forms with action-xhr and not action) for different reasons compared to websites, so we'd probably need a separate discussion on whether to change this.

This is just my opinion. We'll discuss in the next WG meeting and report back.


Unrelated to that, does amp-form actually support serializing files when making an XHR? We may need to confirm that before enabling it. I suspect it'd just not include file data into the POST request, but haven't checked.

@fstanis if possible, it would be nice to support proxying this for consistency in formats as much as we can. It should follow the exact same code path as <img src>, which makes me think implementation would be straightforward.

I agree, but the issue is that:

  • We need to make sure all email clients supporting AMP are (1) aware of this and (2) are appropriately handling it. We can use the WG meeting to coordinate this effort, but I don't want to block this issue on it, so it's best to just skip email support for now.

  • More things to proxy raises the bar to support AMP for new email clients.

@fstanis I don't understand your earlier comment on type=password and how it relates to action vs. action-xhr. My understanding is that the only difference between type=text and type=password is the way that the browser displays the characters as the user enters them, where password obscures the input.

Oh, I just misinterpreted our docs when I read that:
image

I thought this meant type=password is OK for <form method=POST action> (but not <form method=POST action-xhr>), but later realized it's the opposite (and also that <form method=POST action> is actually not allowed anyway).

TLDR: Ignore my previous comment regarding password, it's not allowed in emails for different reasons (phishing concerns).

Another example of input[type=image] causing an issue. The PayPal donation form snippet apparently uses this, causing an unexpected result in AMP. Support forum topic: https://wordpress.org/support/topic/paypal-form-in-reader-mode-not-visible/

FYI, support for input[type=button] was added in https://github.com/ampproject/amphtml/pull/26845

Aside: I've actually noticed a small number of documents that appear to have autoconverted input[type=button] to button, likely due to this. This is error-prone because input[type=button] has no closing HTML tag, while button does, so some of these conversions have broken the page in a major way with unclosed button tags.

input[type=image] still requires additional work, and I'm disappointed that it will end up a allowed in AMP, but not AMP Email.

input[type=password] is not considered acceptable for AMP Email due to phishing concerns. There are also no examples shared here where there is demand for this, it appears mentioned only out of completeness. I'm inclined to leave this one disallowed in all formats for the time being.

input[type=password] is not considered acceptable for AMP Email due to phishing concerns. There are also no examples shared here where there is demand for this, it appears mentioned only out of completeness. I'm inclined to leave this one disallowed in all formats for the time being.

Disallowed in all formats? It is currently allowed inside form[method=post] for the AMP format exclusively, right?

https://github.com/ampproject/amphtml/blob/d32bf3b3751b1184b1d46b20e67a3f569cd93313/validator/validator-main.protoascii#L4558-L4576

Was this page helpful?
0 / 5 - 0 ratings