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: Latest HTML position trumps latest entry #138

Closed
wants to merge 1 commit into from

Conversation

ftes
Copy link
Contributor

@ftes ftes commented Oct 9, 2024

Looking at #137 I realised we probably have a bug (already prior to #137).

Todo

  • Add failing test
  • Fix

Given

<label>Email duplicate 1<input id="email-duplicate-1" name="email" /></label>
<label>Email duplicate 2<input id="email-duplicate-2" name="email" /></label>
|> fill_in("Email duplicate 2", with: "Latest HTML position")
|> fill_in("Email duplicate 1", with: "Latest fill_in")

Expected

# POST email=Latest fill_in&email=Latest HTML position
params = %{"email" => "Latest HTML position"}

Actual

params = %{"email" => "Latest fill_in"}

Background

list of all the submittable elements whose form owner is form, in tree order
https://www.w3.org/TR/2011/WD-html5-20110525/association-of-controls-and-forms.html#constructing-form-data-set

@ftes ftes force-pushed the fix/latest-html-pos branch from decc8a4 to ae70cb3 Compare October 9, 2024 15:35
@ftes
Copy link
Contributor Author

ftes commented Oct 9, 2024

Unrelated, but a recurring thought:

Adding a JS driver will help prevent some of these HTML-semantics-bugs.
Because running the existing tests with an actual browser shows that at least for those cases we are HTML-spec-conformant.

Along the same lines I've found a few edge case bugs while working on #136 and #74 because existing tests were unexpectedly failing.

@ftes
Copy link
Contributor Author

ftes commented Dec 13, 2024

I'll close this for now. Not too common a problem, no easy fix.
But maybe something for back of mind.

@ftes ftes closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant