Track hash changes in session history #21048

cbrewster · 2018-06-13T20:24:25Z

Adds tracking of hash changes in the session history.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #14970 fix #13437 (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2018-06-13T20:24:30Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/history.rs, components/constellation/constellation.rs, components/script/dom/window.rs, components/constellation/session_history.rs
@paulrouget: components/constellation/constellation.rs, components/constellation/session_history.rs
@KiChjang: components/script/dom/history.rs, components/script_traits/script_msg.rs, components/script/dom/window.rs

cbrewster · 2018-06-13T20:28:02Z

r? @asajeffrey

jdm · 2018-06-13T23:51:57Z

tests/wpt/metadata/html/browsers/browsing-the-web/history-traversal/hashchange_event.html.ini

  [Queue a task to fire hashchange event]
-    expected: TIMEOUT
+    expected: FAIL


Why do we fail this test?

This test expects bubbles for the hashchange event to be false, but /html/browsers/browsing-the-web/scroll-to-fragid/004.html expects bubbles to be true. I wasn't able to find where this is specified in the specification, but Firefox sets bubbles to be true, so I went with that. One of the tests will need to be fixed.

Interestingly, Firefox passes both tests.

Also it looks like bubbles should be false per the "Example" block at https://dom.spec.whatwg.org/#concept-event-fire.

Given web-platform-tests/wpt#11355 from 9 days ago, I think we should update 004.html to match.

Ahh that explains it! I'll update it to false

bors-servo · 2018-06-23T03:54:46Z

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

paulrouget · 2018-06-25T00:54:37Z

Does this trigger the HistoryChanged embedder message?

cbrewster · 2018-06-27T10:10:04Z

@paulrouget looks like it doesn't. I don't think history.pushState does either, so I will need to fix both of those.

servo-wpt-sync · 2018-06-27T21:05:48Z

Error syncing changes upstream. Logs saved in error-snapshot-1530134754172.

bors-servo · 2018-07-19T02:56:07Z

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

asajeffrey · 2018-07-20T14:07:10Z

OK, this looks really good! Squash, rebase and r=me.

cbrewster · 2018-07-20T19:00:12Z

@bors-servo r=asajeffrey

bors-servo · 2018-07-20T19:00:13Z

📌 Commit 33f40a4 has been approved by asajeffrey

bors-servo · 2018-07-20T19:00:18Z

⌛ Testing commit 33f40a4 with merge ce0014b...


        Auto merge of #21048 - cbrewster:hash_change_history, r=asajeffrey

Track hash changes in session history  Adds tracking of hash changes in the session history. ---  - [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).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048)

bors-servo · 2018-07-20T19:04:39Z

💔 Test failed - linux-rel-css

jdm · 2018-07-20T19:20:43Z

error[E0061]: this function takes 1 parameter but 0 parameters were supplied
    --> components/script/dom/window.rs:1612:39
     |
1087 |     pub fn task_canceller(&self, name: TaskSourceName) -> TaskCanceller {
     |     ------------------------------------------------------------------- defined here
...
1612 |                         Box::new(self.task_canceller().wrap_task(task)),
     |                                       ^^^^^^^^^^^^^^ expected 1 parameter

error: aborting due to previous error

jdm · 2018-07-20T20:53:32Z

  ▶ Unexpected subtest result in /css/cssom-view/scroll-behavior-smooth.html:
  └ PASS [expected NOTRUN] Smooth scrolling while doing history navigation.

  ▶ Unexpected subtest result in /css/cssom-view/scroll-behavior-smooth.html:
  │ FAIL [expected TIMEOUT] Instant scrolling while doing history navigation.
  │   → assert_equals: Shouldn't be scrolled back to top yet. expected 0 but got 28739
  │ 
  │ instant/window.onhashchange/window.onhashchange/<@http://web-platform.test:8000/css/cssom-view/scroll-behavior-smooth.html:80:11
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1538:20
  └ instant/window.onhashchange/window.onhashchange@http://web-platform.test:8000/css/cssom-view/scroll-behavior-smooth.html:78:9


        Track hash changes in session history

Notify history changed on pushState and scroll to frag

cbrewster · 2018-07-21T18:39:09Z

@bors-servo r+

bors-servo · 2018-07-21T18:39:10Z

📌 Commit 61442cc has been approved by cbrewster

bors-servo · 2018-07-21T18:39:14Z

⌛ Testing commit 61442cc with merge 91c8f26...


        Auto merge of #21048 - cbrewster:hash_change_history, r=cbrewster

Track hash changes in session history  Adds tracking of hash changes in the session history. ---  - [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).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048)

cbrewster · 2018-07-21T18:39:21Z

@bors-servo r=asajeffrey

bors-servo · 2018-07-21T18:39:22Z

💡 This pull request was already approved, no need to approve it again.

This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

bors-servo · 2018-07-21T18:39:22Z

📌 Commit 61442cc has been approved by asajeffrey

bors-servo · 2018-07-21T18:39:27Z

⌛ Testing commit 61442cc with merge a97d8b9...


        Auto merge of #21048 - cbrewster:hash_change_history, r=asajeffrey

Track hash changes in session history  Adds tracking of hash changes in the session history. ---  - [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).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21048)

bors-servo · 2018-07-21T19:38:22Z

💔 Test failed - mac-rel-wpt3

cbrewster · 2018-07-21T20:06:18Z

@bors-servo retry

websockets

bors-servo · 2018-07-21T20:06:21Z

⚡ Previous build results for android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt4, windows-msvc-dev are reusable. Rebuilding only mac-rel-wpt3...

bors-servo · 2018-07-21T20:48:53Z

☀️ Test successful - android, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: asajeffrey
Pushing a97d8b9 to master...

highfive assigned mbrubeck Jun 13, 2018

highfive added the S-awaiting-review label Jun 13, 2018

highfive assigned asajeffrey and unassigned mbrubeck Jun 13, 2018

jdm reviewed Jun 13, 2018

View changes

highfive added the S-needs-rebase label Jun 23, 2018

cbrewster force-pushed the cbrewster:hash_change_history branch from d0a7cc2 to b6b0f32 Jun 27, 2018

cbrewster removed the S-needs-rebase label Jun 27, 2018

highfive added the S-needs-rebase label Jul 19, 2018

cbrewster force-pushed the cbrewster:hash_change_history branch from b6b0f32 to 33f40a4 Jul 20, 2018

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Jul 20, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 20, 2018

cbrewster force-pushed the cbrewster:hash_change_history branch from 33f40a4 to 39acdcd Jul 20, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 20, 2018

Track hash changes in session history

Loading status checks…

61442cc

Notify history changed on pushState and scroll to frag

cbrewster force-pushed the cbrewster:hash_change_history branch from 39acdcd to 61442cc Jul 21, 2018

highfive added S-awaiting-review and removed S-tests-failed labels Jul 21, 2018

highfive assigned cbrewster and unassigned asajeffrey Jul 21, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 21, 2018

highfive assigned asajeffrey and unassigned cbrewster Jul 21, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 21, 2018

bors-servo mentioned this pull request Jul 21, 2018

Sync WPT with upstream (20-07-2018) #21220

Closed

servo / servo

Track hash changes in session history #21048

Track hash changes in session history #21048

cbrewster commented Jun 13, 2018 •

edited by atouchet

highfive commented Jun 13, 2018

cbrewster commented Jun 13, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Jun 23, 2018

paulrouget commented Jun 25, 2018

cbrewster commented Jun 27, 2018

servo-wpt-sync commented Jun 27, 2018

bors-servo commented Jul 19, 2018

asajeffrey commented Jul 20, 2018

cbrewster commented Jul 20, 2018

bors-servo commented Jul 20, 2018

bors-servo commented Jul 20, 2018

bors-servo commented Jul 20, 2018

jdm commented Jul 20, 2018

jdm commented Jul 20, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

servo / servo

Join GitHub today

Track hash changes in session history #21048

Track hash changes in session history #21048

Conversation

cbrewster commented Jun 13, 2018 • edited by atouchet

highfive commented Jun 13, 2018

cbrewster commented Jun 13, 2018

This comment has been minimized.

jdm Jun 13, 2018

This comment has been minimized.

cbrewster Jun 14, 2018

This comment has been minimized.

jdm Jun 14, 2018

This comment has been minimized.

jdm Jun 14, 2018

This comment has been minimized.

jdm Jun 14, 2018

This comment has been minimized.

cbrewster Jun 27, 2018

bors-servo commented Jun 23, 2018

paulrouget commented Jun 25, 2018

cbrewster commented Jun 27, 2018

servo-wpt-sync commented Jun 27, 2018

bors-servo commented Jul 19, 2018

asajeffrey commented Jul 20, 2018

cbrewster commented Jul 20, 2018

bors-servo commented Jul 20, 2018

bors-servo commented Jul 20, 2018

bors-servo commented Jul 20, 2018

jdm commented Jul 20, 2018

jdm commented Jul 20, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

cbrewster commented Jul 21, 2018

bors-servo commented Jul 21, 2018

bors-servo commented Jul 21, 2018

cbrewster commented Jun 13, 2018 •

edited by atouchet