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

Implement Window.open and related infrastructure #20678

Merged
merged 7 commits into from Aug 11, 2018

Conversation

Projects
None yet
@gterzian
Collaborator

gterzian commented Apr 22, 2018

Implement https://html.spec.whatwg.org/multipage/window-object.html#window-open-steps and related infra...


  • ./mach build -d does not report any errors
  • ./mach build-geckolib does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #20673 fix #13241 fix #20887 fix #20713 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive

This comment has been minimized.

highfive commented Apr 22, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmlanchorelement.rs
  • @fitzgen: components/script/dom/htmlanchorelement.rs
  • @KiChjang: components/script/dom/htmlanchorelement.rs
@highfive

This comment has been minimized.

highfive commented Apr 22, 2018

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

@gterzian gterzian changed the title from [WIP] Improve spec compliance of "follow hyperlinks" to [WIP] Implement "the rules for choosing a browsing context" Apr 22, 2018

@servo-wpt-sync

This comment has been minimized.

Collaborator

servo-wpt-sync commented Apr 22, 2018

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at jdm/web-platform-tests#51.

@gterzian gterzian force-pushed the gterzian:improve_follow_hyperlinks branch from 6a2d887 to bc26359 Apr 22, 2018

@servo-wpt-sync

This comment has been minimized.

Collaborator

servo-wpt-sync commented Apr 22, 2018

No upstreamable changes; closed existing PR.

Completed upstream sync of web-platform-test changes at jdm/web-platform-tests#51.

@gterzian gterzian force-pushed the gterzian:improve_follow_hyperlinks branch 6 times, most recently from 1bd9f3a to f594279 Apr 23, 2018

@gterzian gterzian changed the title from [WIP] Implement "the rules for choosing a browsing context" to Implement "the rules for choosing a browsing context" Apr 23, 2018

@gterzian gterzian force-pushed the gterzian:improve_follow_hyperlinks branch from dc94a69 to 4b2b6e8 Apr 23, 2018

@gterzian

This comment has been minimized.

Collaborator

gterzian commented Apr 23, 2018

This one is now ready for review.

A few notes:

  1. Why return a Window from choose_browsing_context? Because although we are choosing a browsing context, it is for the purpose of accessing the capabilities of the related window(such as navigation)
  2. Why put choose_browsing_context on WindowProxy? Because it seems that it has all the info necessary for choosing a browsing context.

@gterzian gterzian force-pushed the gterzian:improve_follow_hyperlinks branch 4 times, most recently from cb33891 to 32095f4 Apr 23, 2018

@gterzian

This comment has been minimized.

Collaborator

gterzian commented Apr 27, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Apr 27, 2018

@gterzian: 🔑 Insufficient privileges: and not in try users

@cbrewster

This comment has been minimized.

Member

cbrewster commented Apr 27, 2018

@bors-servo try

Sorry homu has to be restarted before it takes the new changes into effect.

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Apr 27, 2018

⌛️ Trying commit 32095f4 with merge ad4c78e...

bors-servo added a commit that referenced this pull request Apr 27, 2018

Auto merge of #20678 - gterzian:improve_follow_hyperlinks, r=<try>
Implement "the rules for choosing a browsing context"

<!-- Please describe your changes on the following line: -->

Implement https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name

and plug into
1. https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks
2. https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-form-submit

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach build-geckolib` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix  #20673 (github issue number if applicable).

<!-- Either: -->
- [ ] 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/20678)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Apr 27, 2018

💔 Test failed - linux-rel-css

@servo-wpt-sync

This comment has been minimized.

Collaborator

servo-wpt-sync commented Aug 10, 2018

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at jdm/web-platform-tests#76.

@gterzian

This comment has been minimized.

Collaborator

gterzian commented Aug 10, 2018

@jdm OK those were indeed too quickly set to PASS, I've set them back to FAIL...

{"status": "FAIL", "group": "default", "message": "assert_array_equals: Pages opened during history navigation property 1, expected 3 but got 2", "stack": "start_test_wait</timer<@http://web-platform.test:8000/html/browsers/history/the-history-interface/traverse_the_history_2.html:17:13\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1553:20\nTest.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1577:20\n", "subtest": "Multiple history traversals, last would be aborted", "test": "/html/browsers/history/the-history-interface/traverse_the_history_2.html", "line": 84689, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "assert_array_equals: Pages opened during history navigation property 1, expected 5 but got 3", "stack": "start_test_wait</timer<@http://web-platform.test:8000/html/browsers/history/the-history-interface/traverse_the_history_4.html:17:13\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1553:20\nTest.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1577:20\n", "subtest": "Multiple history traversals, last would be aborted", "test": "/html/browsers/history/the-history-interface/traverse_the_history_4.html", "line": 119587, "action": "test_result", "expected": "PASS"}

while this looks like an intermittent

{"status": "FAIL", "group": "default", "message": "assert_equals: expected \"href\" but got \"click\"", "stack": "onmessage<@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/006.html:14:5\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1553:20\nTest.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1577:20\n", "subtest": "Link with onclick form submit and href navigation ", "test": "/html/browsers/browsing-the-web/navigating-across-documents/006.html", "line": 68859, "action": "test_result", "expected": "PASS"}
@jdm

This comment has been minimized.

Member

jdm commented Aug 10, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 10, 2018

📌 Commit e784f5a has been approved by jdm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 10, 2018

⌛️ Testing commit e784f5a with merge 9c24c16...

bors-servo added a commit that referenced this pull request Aug 10, 2018

Auto merge of #20678 - gterzian:improve_follow_hyperlinks, r=jdm
Implement Window.open and related infrastructure

<!-- Please describe your changes on the following line: -->

Implement https://html.spec.whatwg.org/multipage/window-object.html#window-open-steps and related infra...

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach build-geckolib` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix  #20673 fix #13241 fix #20887 fix #20713 (github issue number if applicable).

<!-- Either: -->
- [ ] 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/20678)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 11, 2018

💔 Test failed - linux-rel-css

@jdm

This comment has been minimized.

Member

jdm commented Aug 11, 2018

@bors-servo retry

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 11, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 11, 2018

💔 Test failed - linux-rel-css

@CYBAI

This comment has been minimized.

Collaborator

CYBAI commented Aug 11, 2018

@bors-servo retry

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 11, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 11, 2018

@bors-servo bors-servo merged commit e784f5a into servo:master Aug 11, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@servo-wpt-sync

This comment has been minimized.

Collaborator

servo-wpt-sync commented Aug 11, 2018

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

@paulrouget

This comment has been minimized.

Contributor

paulrouget commented Aug 11, 2018

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment