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

Workarounds for form-submission-0 tests with about:blank onloads #25220

Merged
merged 1 commit into from Dec 10, 2019

Conversation

@pshaughn
Copy link
Member

pshaughn commented Dec 9, 2019

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)


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25151 and fix #25152.
  • These changes do not require tests because these are fixes to tests only, not code changes
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 9, 2019

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#20686.

async_test(t => {
frame1.onload = t.step_func_done(() => {
assert_not_equals(frame1.contentDocument.URL.indexOf('_charset_=windows-1252'), -1);
let tCharset=async_test('_charset_ control sets the expected encoding name.');

This comment has been minimized.

Copy link
@jdm

jdm Dec 9, 2019

Member

Let's keep the existing structure like so:

async_test(t => {
  frame1.onload = t.step_func(() => {
    if (frame1.contentWindow.location.href == "about:blank") { return; }
    assert_not_equals(frame1.contentDocument.URL.indexOf('_charset_=windows-1252'), -1);
    tCharset.done();
  };
}, '_charset_ control sets the expected encoding name.');
@jdm jdm assigned jdm and unassigned ferjm Dec 9, 2019
@pshaughn pshaughn force-pushed the pshaughn:patch25151 branch from 81454fa to 6960f72 Dec 9, 2019
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 9, 2019

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#20686.

@jdm
Copy link
Member

jdm commented Dec 9, 2019

Running the WPT lint on ./tests/wpt/web-platform-tests/...
{"path": "html/semantics/forms/form-submission-0/form-submission-algorithm.html", "message": "Whitespace at EOL", "lineno": 70, "rule": "TRAILING WHITESPACE"}
{"path": "html/semantics/forms/form-submission-0/form-submission-algorithm.html", "message": "Whitespace at EOL", "lineno": 138, "rule": "TRAILING WHITESPACE"}
 0:10.63�(B INFO�(B Diffing old and new manifests /repo/tests/wpt/metadata/MANIFEST.json
 0:16.02�(B WARNING�(B Manifest /repo/tests/wpt/metadata/MANIFEST.json contains correct tests but file hashes changed.
 0:16.13�(B ERROR�(B Manifest /repo/tests/wpt/metadata/MANIFEST.json is outdated, use |./mach update-manifest| to fix.

Otherwise this looks good!

@pshaughn pshaughn force-pushed the pshaughn:patch25151 branch from 6960f72 to 6d018f4 Dec 9, 2019
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 9, 2019

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#20686.

@jdm
Copy link
Member

jdm commented Dec 9, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2019

📌 Commit 6d018f4 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

Testing commit 6d018f4 with merge 11a30f1...

bors-servo added a commit that referenced this pull request 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 10, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

The latest upstream changes (presumably #25217) made this pull request unmergeable. Please resolve the merge conflicts.

@pshaughn pshaughn force-pushed the pshaughn:patch25151 branch from 6d018f4 to 15e5aef Dec 10, 2019
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Dec 10, 2019

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#20686.

@jdm
Copy link
Member

jdm commented Dec 10, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

📌 Commit 15e5aef has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

Testing commit 15e5aef with merge b19b778...

bors-servo added a commit that referenced this pull request 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing b19b778 to master...

@bors-servo bors-servo merged commit 15e5aef into servo:master Dec 10, 2019
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.