Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uphashchange DOM event should be dispatched when the document location's hash is changed #14970
Comments
|
cc @esprehn |
|
I've observed this a few times after variable numbers of tests have completed. I suspect there's a timing issue that makes the harness get confused, rather than an internal engine problem like a deadlock. |
|
The Mithril suite seems to trigger this most often when I run the tests in isolation. |
|
Ractive triggered the JS errors for me. |
|
Those both seem to be consistent for me, and it would explain the intermittent behaviour you observed - the order of the testsuites seems to be randomized every time I load the benchmark. |
|
I filed #14975 for the Ractive problem. |
|
The problem with the tests not continuing is due to the Mithril test suite, and that's caused by us missing the |
|
To reproduce easily, run an HTTP server inside todomvc-perf-comparison/todomvc-benchmark, and run |
|
@cbrewster The spec text for hashchange falls under steps 6 and 15 of https://html.spec.whatwg.org/multipage/browsers.html#traverse-the-history . Do we have an implementation of the "traverse the history" algorithm yet? |
|
@jdm the "traverse the history" algorithm is found in the constellation: |
Track hash changes in session history <!-- Please describe your changes on the following line: --> Adds tracking of hash changes in the session history. --- <!-- 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 #14970 fix #13437 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048) <!-- Reviewable:end -->
Track hash changes in session history <!-- Please describe your changes on the following line: --> Adds tracking of hash changes in the session history. --- <!-- 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 #14970 fix #13437 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048) <!-- Reviewable:end -->
Track hash changes in session history <!-- Please describe your changes on the following line: --> Adds tracking of hash changes in the session history. --- <!-- 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 #14970 fix #13437 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048) <!-- Reviewable:end -->
Track hash changes in session history <!-- Please describe your changes on the following line: --> Adds tracking of hash changes in the session history. --- <!-- 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 #14970 fix #13437 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- 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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048) <!-- Reviewable:end -->
The tests will start running. At some point during the benchmark, execution stops and nothing further happens. The rendering is still running (can be confirmed with -Z wr-stats).
Sometimes you get several JS errors written on the console - but this doesn't always happen.
For example: