History interface Go, Back, and Forward #12552

cbrewster · 2016-07-22T04:49:09Z

r? @asajeffrey

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

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

implement go, forward, back

This change is

highfive · 2016-07-22T04:49:12Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/history.rs, components/script/dom/webidls/History.webidl, components/script/dom/mod.rs, components/script/dom/window.rs, components/script/dom/webidls/Window.webidl

cbrewster · 2016-07-22T04:49:21Z

@bors-servo try

bors-servo · 2016-07-22T04:49:23Z

⌛ Trying commit 09ecfbb with merge 80d469d...


        Auto merge of #12552 - ConnorGBrewster:history_interface, r=<try>

@asajeffrey

History interface Go, Back, and Forward  r? @asajeffrey ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #5670 (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____  implement go, forward, back

bors-servo · 2016-07-22T05:46:58Z

💔 Test failed - linux-rel

asajeffrey · 2016-07-22T14:49:18Z


Tests with unexpected results:
  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html:
  │ FAIL [expected PASS] Restoring window.name on cross-origin history traversal
  │   → f.contentWindow is null
  │ 
  │ steps<@http://web-platform.test:8000/html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html:13:17
  │ steps</<@http://web-platform.test:8000/html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html:25:72
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1402:20
  │ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1426:20
  │ next<@http://web-platform.test:8000/html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html:28:32
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1402:20
  └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1426:20

  ▶ Unexpected subtest result in /html/browsers/history/the-location-interface/location_assign_about_blank.html:
  └ PASS [expected FAIL] location.assign with initial about:blank browsing context

  ▶ Unexpected subtest result in /_mozilla/mozilla/interfaces.html:
  │ FAIL [expected PASS] Interfaces exposed on the window
  │   → assert_true: If this is failing: DANGER, are you sure you want to expose the new interface History to all webpages as a property on the global? Do not make a change to this file without review from jdm or Ms2ger for that specific change! expected true got false
  │ 
  │ test_interfaces/<@http://web-platform.test:8000/_mozilla/mozilla/interfaces.js:70:7
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1402:20
  │ test@http://web-platform.test:8000/resources/testharness.js:500:9
  │ test_interfaces@http://web-platform.test:8000/_mozilla/mozilla/interfaces.js:1:3
  └ @http://web-platform.test:8000/_mozilla/mozilla/interfaces.html:6:1

  ▶ Unexpected subtest result in /_mozilla/mozilla/interfaces.worker:
  │ FAIL [expected PASS] Untitled
  │   → assert_true: If this is failing: DANGER, are you sure you want to expose the new interface History to all webpages as a property on the global? Do not make a change to this file without review from jdm or Ms2ger for that specific change! expected true got false
  │ 
  │ test_interfaces/<@http://web-platform.test:8000/_mozilla/mozilla/interfaces.js:71:7
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1403:20
  │ test@http://web-platform.test:8000/resources/testharness.js:501:9
  │ test_interfaces@http://web-platform.test:8000/_mozilla/mozilla/interfaces.js:2:3
  └ @http://web-platform.test:8000/_mozilla/mozilla/interfaces.worker.js:9:1

The first fail is the interesting one, the second is good news. The other two are just reminding us to get @jdm or @Ms2ger to bless this PR.

cbrewster · 2016-07-22T15:37:10Z

Updated test expectations

asajeffrey · 2016-07-22T15:44:15Z

Reviewed 13 of 14 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/webidls/History.webidl, line 10 [r2] (raw file):

[Exposed=(Window,Worker)]
interface History {
  // readonly attribute unsigned long length;

It would be nice to support length at some point, not necessarily this PR though.

Comments from Reviewable

asajeffrey · 2016-07-22T15:45:51Z

LGTM, we need @jdm or @Ms2ger to bless it, then you can r=me.

jdm · 2016-07-22T15:52:40Z

History does not belong in worker scopes and shouldn't have any Exposed annotation.

asajeffrey · 2016-07-22T16:08:17Z

IRC conversation: http://logs.glob.uno/?c=mozilla%23servo&s=22+Jul+2016&e=22+Jul+2016#c486162

asajeffrey · 2016-07-22T16:09:50Z

@jdm blesses http://logs.glob.uno/?c=mozilla%23servo&s=22+Jul+2016&e=22+Jul+2016#c486278 so @bors-servo r+

bors-servo · 2016-07-22T16:09:51Z

📌 Commit 7273f24 has been approved by asajeffrey

bors-servo · 2016-07-22T16:10:24Z

⌛ Testing commit 7273f24 with merge 445152c...


        Auto merge of #12552 - ConnorGBrewster:history_interface, r=asajeffrey

@asajeffrey

History interface Go, Back, and Forward  r? @asajeffrey ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #5670 (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____  implement go, forward, back  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12552)

bors-servo · 2016-07-22T17:07:39Z

💔 Test failed - linux-rel

cbrewster · 2016-07-22T17:18:58Z

@bors-servo retry

#12561

bors-servo · 2016-07-22T18:15:15Z

⌛ Testing commit 7273f24 with merge 629675a...


        Auto merge of #12552 - ConnorGBrewster:history_interface, r=asajeffrey

@asajeffrey

History interface Go, Back, and Forward  r? @asajeffrey ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #5670 (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____  implement go, forward, back  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12552)

bors-servo · 2016-07-22T19:11:29Z

💔 Test failed - linux-rel

highfive · 2016-07-22T19:11:32Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html:
  └ PASS [expected FAIL] Restoring window.name on cross-origin history traversal

asajeffrey · 2016-07-22T19:16:30Z

Oh how ironic, this test is now intermittent, most likely due to the race condition between f.onload and the setTimeout.

cbrewster · 2016-07-22T19:27:43Z

@asajeffrey sigh.. I wonder if there is a way we can make this pass in both Gecko and Servo regardless of the onload issue

cbrewster · 2016-07-22T19:43:11Z

Would it be ok to change: https://github.com/ConnorGBrewster/servo/blob/master/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/history-traversal/browsing_context_name_cross_origin.html#L40

onload = setTimeout(next, 0);

to

next();

That makes it pass in Servo(as the onload callback is not called for about:blank) and I assume Gecko as well (Firefox gets mad about www.localhost and I don't have a local copy of Gecko, it would be nice if someone could double check this).


        Add History interface

implement go, forward, back

cbrewster · 2016-07-22T20:13:06Z

Disabled browsing_context_name_cross_origin.html until we can get a fix for it.

cbrewster · 2016-07-22T20:34:11Z

@bors-servo r=asajeffrey

bors-servo · 2016-07-22T20:34:13Z

📌 Commit 8d7a0c2 has been approved by asajeffrey

bors-servo · 2016-07-22T20:34:15Z

⌛ Testing commit 8d7a0c2 with merge 0e887ca...


        Auto merge of #12552 - ConnorGBrewster:history_interface, r=asajeffrey

@asajeffrey

History interface Go, Back, and Forward  r? @asajeffrey ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #5670 (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____  implement go, forward, back  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12552)

bors-servo · 2016-07-22T21:31:36Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

emilio · 2016-07-26T11:10:32Z

#12563 makes some changes to onload to make it more consistent, would be nice to see if those changes fix any of the races you see here.

cbrewster · 2016-08-01T14:47:43Z

@emilio I tested locally and haven't hit any intermittent failures! 🎉 I will leave those issues open for a bit to make sure. Thanks!

highfive assigned asajeffrey Jul 22, 2016

highfive added the S-awaiting-review label Jul 22, 2016

highfive added the S-tests-failed label Jul 22, 2016

cbrewster force-pushed the cbrewster:history_interface branch from 09ecfbb to bec5a95 Jul 22, 2016

highfive removed the S-tests-failed label Jul 22, 2016

cbrewster mentioned this pull request Jul 22, 2016

History API metabug #10992

Open

18 of 24 tasks complete

cbrewster force-pushed the cbrewster:history_interface branch from bec5a95 to 7273f24 Jul 22, 2016

asajeffrey added S-awaiting-answer and removed S-awaiting-review labels Jul 22, 2016

highfive added S-awaiting-merge and removed S-awaiting-answer labels Jul 22, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 22, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Jul 22, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jul 22, 2016

Add History interface

Loading status checks…

8d7a0c2

implement go, forward, back

cbrewster force-pushed the cbrewster:history_interface branch from 7273f24 to 8d7a0c2 Jul 22, 2016

highfive added S-awaiting-review and removed S-tests-failed labels Jul 22, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 22, 2016

bors-servo merged commit 8d7a0c2 into servo:master Jul 22, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jul 22, 2016

bors-servo mentioned this pull request Jul 22, 2016

Partial fix for #12415: expose interfaces only where necessary #12537

Merged

3 of 3 tasks complete

stshine mentioned this pull request Jul 26, 2016

Implement flexible box layout for row container #12330

Merged

3 of 3 tasks complete

asajeffrey mentioned this pull request May 12, 2017

No support for History APIs #5669

Closed

servo / servo

History interface Go, Back, and Forward #12552

History interface Go, Back, and Forward #12552

cbrewster commented Jul 22, 2016 •

edited by larsbergstrom

highfive commented Jul 22, 2016

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

cbrewster commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

jdm commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

highfive commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

cbrewster commented Jul 22, 2016

cbrewster commented Jul 22, 2016 •

edited

cbrewster commented Jul 22, 2016 •

edited

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

emilio commented Jul 26, 2016

cbrewster commented Aug 1, 2016

servo / servo

Join GitHub today

History interface Go, Back, and Forward #12552

History interface Go, Back, and Forward #12552

Conversation

cbrewster commented Jul 22, 2016 • edited by larsbergstrom

highfive commented Jul 22, 2016

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

cbrewster commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

jdm commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

highfive commented Jul 22, 2016

asajeffrey commented Jul 22, 2016

cbrewster commented Jul 22, 2016

cbrewster commented Jul 22, 2016 • edited

cbrewster commented Jul 22, 2016 • edited

cbrewster commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

bors-servo commented Jul 22, 2016

emilio commented Jul 26, 2016

cbrewster commented Aug 1, 2016

cbrewster commented Jul 22, 2016 •

edited by larsbergstrom

cbrewster commented Jul 22, 2016 •

edited

cbrewster commented Jul 22, 2016 •

edited