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

_charset_ hidden field isn't working in WPT html/semantics/forms/form-submission-0/constructing-form-data-set.html #25151

Closed
pshaughn opened this issue Dec 6, 2019 · 7 comments

Comments

@pshaughn
Copy link
Member

@pshaughn pshaughn commented Dec 6, 2019

script/dom/htmlformelement.rs does have special-case code for charset as required by https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#naming-form-controls:-the-name-attribute but in this test it isn't working for some reason.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 6, 2019

Possibly this is a timing issue in which the target frame's location hasn't updated to include the form GET when we look at it.

on second look, no, it's an onload, the location has to have changed by now.

@jdm jdm added the A-content/dom label Dec 6, 2019
@jdm jdm added this to To do in web-platform-test failures via automation Dec 6, 2019
@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 9, 2019

@highfive assign me

@highfive
Copy link

@highfive highfive commented Dec 9, 2019

Hey @pshaughn! Thanks for your interest in working on this issue. It's now assigned to you!

@highfive highfive added the C-assigned label Dec 9, 2019
@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 9, 2019

My earlier guess about the target frame's location wasn't as wrong as I thought it was.
This test's frame1 is firing an onload even when I comment out the form.submit(), that onload has its url as about:blank, and including the form.submit() does not lead to a second onload.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 9, 2019

This differs from #25154 in that the unwanted load is happening here even though the frame's src is never explicitly set.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 9, 2019

3b9ab34#diff-01b44abc6218a3ab6d34cd50e43440fd shows that something like this has happened before; I'll try copying that workaround!

update: the workaround works to move the test forward, I am now testing the correct onload (which does indeed contain the wrong value)

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 9, 2019

The right thing happens if I don't touch the Rust code at all; it's happening in the wrong place relative to WHATWG specs, but it's passing the fixed test.

bors-servo added a commit that referenced this issue Dec 10, 2019
Workarounds for form-submission-0 tests with about:blank onloads

<!-- Please describe your changes on the following line: -->
Following the model of 3b9ab34 these changes replace some naive onload events with equivalents that do nothing if they see an about:blank load event. Two test cases now pass, and one failing test case is now failing for a less ambiguous reason (#25154)

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25151 and fix #25152.

<!-- Either: -->
- [X] These changes do not require tests because these are fixes to tests only, not code changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
web-platform-test failures automation moved this from To do to Done Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.