Amphtml: amp-form: omit target attribute for POST requests

[sebastianbenz]

POST requests are only allowed via XHR (link. However, the validator still requires action and target to be set.

=> If this is the intended (no JS fallback) behavior, we should make this explicit in the documentation (and samples).
=> If this is not the intended behavior, we should update the validation rules.

//cc @mkhatib @Gregable

Context: PR #5548

[dandv]

Ran into the same error - https://github.com/ampproject/amp-by-example/issues/426

It doesn't quite make sense to specify a target for XHR requests that update the same page.

[mkhatib]

Agreed. @Gregable I am not sure if we discussed this before, but would conditional requirement of an attribute be possible in the validator? So only require target for method=GET?

This also needs to be updated in runtime.

[mkhatib]

@cramforce does this make sense? I remember we wanted to enforce having target all the time at first, but since our implementation changed - do you mind if we just drop the requirement for target for xhr requests?

[cramforce]

@mkhatib no target seems fine, even better.

[mkhatib]

Ok so thinking of making target only required for non-XHR GET requests. So only if action-xhr == undefined and method=GET

[cramforce]

No target, just means target is _top. No need to make it explicit I think.

On Tue, Oct 18, 2016 at 1:00 PM, Mohammad Khatib [email protected]
wrote:

Ok so thinking of making target only required for non-XHR GET requests.
So only if action-xhr == undefined and method=GET

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ampproject/amphtml/issues/5607#issuecomment-254622072,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFeT76pIGWPTckBAjtVKScarL0CfrRvks5q1SV4gaJpZM4KWqUo
.

[drdamour]

i was confused by the docs on form because the example has method=POST but also has an action attribute which is defined as invalid, is the sample wrong?

[mkhatib]

Yes apologies for that, I think we had a PR to update the examples in the docs but was not followed up on, will go ahead and update the docs in another PR.

[mkhatib]

@Gregable this is in runtime now too - won't be in production until 2 weeks - so we can probably get started on this on the validator.

[Gregable]

Right now, if a form action is GET, then validation requires:

• target attribute with value of _blank or _top
• action attribute

Right now, if a form action is POST, then validation requires:

• target attribute with value of _blank or _top
• action-xhr attribute

I'm confused as to what, if anything, this still requires. It looks like the thread is requesting that for GET forms, action is always required but only one of target or action-xhr is required. Is that the correct intepretation?

@sebastianbenz , @cramforce

[Stefany93]

@Gregable I think target is no longer mandatory for post requests.

[Gregable]

@Stefany93, good catch. Closing this issue.