Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: form attribute in button doesn't work and throws error #31506

Closed
claviska opened this issue Feb 28, 2022 · 7 comments
Closed

fix: form attribute in button doesn't work and throws error #31506

claviska opened this issue Feb 28, 2022 · 7 comments

Comments

@claviska
Copy link
Contributor

claviska commented Feb 28, 2022

🐛 Bug Report

The documented form attribute on FAST Button is intended to mimic that of a native <button> by allowing the user to pass a form id, effectively reassigning its form owner.

This doesn't work in any browser I tested, and in Chromium results in the following error:

Uncaught DOMException: Failed to execute 'requestSubmit' on 'HTMLFormElement': The specified element is not owned by this form element.
    at pc.handleSubmission (https://unpkg.com/@microsoft/fast-components@2.20.2/dist/fast-components.min.js:16:49579)
handleSubmission @ fast-components.min.js:16

Additionally, it appears as though other form-related attributes including formaction, formmethod, formtarget, etc. do not have any effect in any browser.

💻 Repro or Code Sample

https://jsfiddle.net/d690np3s/1/

🤔 Expected Behavior

In the repro above, the form should be submitted when the button is activated. The form should submit using a GET request to /get in a new tab/window.

😯 Current Behavior

Nothing happens and the aforementioned error is thrown.

💁 Possible Solution

I believe this is happening because the call to requestSubmit() is passing the proxy, but the proxy isn't understood to be "owned" by the form.

I know of two quick ways to solve this:

  1. Add the form attribute to the proxy, e.g. this.proxy.setAttribute("form", this.form.id). This appears to work fine, but relies on the form having an id. Otherwise, the fix will need to account for that by creating a unique id, linking it, and removing it after submission.
  2. Append the proxy to the form before calling requestSubmit(), e.g. this.form.append(proxy), and restore its position as a child of the host element after submission.

However, an even better solution may be to rework the proxy logic so it's aware of linked forms and attaches the proxy to the form instead of the host element when a form attribute exists.

I'm happy to commit some of my team's bandwidth to this if we can get confirmation of the preferred solution.

@EisenbergEffect
Copy link
Contributor

@nicholasrice Any thoughts on the best approach for this?

@nicholasrice
Copy link
Contributor

nicholasrice commented Mar 1, 2022

A bit of context on all of this here (I had to catch myself up on what this code myself): microsoft/fast#3534 (comment)

I think your 1st option is the simplest and most in line with the current solution, though I don't follow the part about creating a form id and then removing it. If the form element doesn't have an id, I'd expect submission would just fail. I think it's enough to simply reflect the form attribute on an FASTButton element to the proxy. If an author mis-configures the FASTButton or doesn't put the id attribute on the form element that's on them to fix.

@claviska
Copy link
Contributor Author

claviska commented Mar 1, 2022

though I don't follow the part about creating a form id and then removing it.

Disregard that part. I had a thought in my head as I was finishing the report and it wasn't fully fleshed out.

Mapping the form id to the proxy's form attribute and letting it fail as it would natively when omitted sounds like the way forward.

@EisenbergEffect
Copy link
Contributor

@claviska If you are interested in contributing the fix, we'd love to have you take that on 😄 Let me know.

@Skintkingle
Copy link

I am interested in seeing this bug get fixed as, via the fluentui-blazor repo it is impacting me. How can I show my dependancy on this bug so it can be prioritised to be fixed in the future?

@janechu janechu transferred this issue from microsoft/fast May 29, 2024
@janechu
Copy link
Contributor

janechu commented May 29, 2024

Transferring this issue as I believe the dependency in fluentui-blazor is now on the fluentui web components.

@chrisdholt
Copy link
Member

Closing this as resolved with #30999 in Beta (though I believe it was also resolved earlier)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants