Skip to content

Add referrer to navigation fetch request #23090

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

Merged
merged 2 commits into from
Apr 26, 2019

Conversation

miller-time
Copy link
Contributor

@miller-time miller-time commented Mar 25, 2019

Implement step 13 of following hyperlinks and step 14.3 of window open, as well as other referrer- and fetch-related updates.


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

This change is Reviewable

@highfive
Copy link

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @nox (or someone else) soon.

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/windowproxy.rs, components/script/dom/htmlanchorelement.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/script/dom/location.rs
  • @KiChjang: components/script/dom/windowproxy.rs, components/script/dom/htmlanchorelement.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/net_traits/request.rs and 2 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 25, 2019
@highfive
Copy link

warning Warning warning

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

@miller-time miller-time force-pushed the nav-fetch-referrer branch 2 times, most recently from b23618d to e98db92 Compare March 30, 2019 23:24
@miller-time miller-time marked this pull request as ready for review March 31, 2019 00:10
@gterzian
Copy link
Member

gterzian commented Apr 4, 2019

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

⌛ Trying commit e98db92 with merge 92a41b9...

bors-servo pushed a commit that referenced this pull request Apr 4, 2019
Add referrer to navigation fetch request

<!-- Please describe your changes on the following line: -->
Implement step 13 of [following hyperlinks](https://html.spec.whatwg.org/#following-hyperlinks-2) and step 14.3 of [window open](https://html.spec.whatwg.org/#window-open-steps), as well as other referrer- and fetch-related updates.

---
<!-- 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 #22890 (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/23090)
<!-- Reviewable:end -->
@gterzian
Copy link
Member

gterzian commented Apr 4, 2019

@miller-time Results from the tests will show up at https://build.servo.org/grid (look for the column matching the commits mentioned above by the bot)

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Apr 4, 2019
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Apr 8, 2019
@bors-servo
Copy link
Contributor

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

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Apr 11, 2019
@miller-time miller-time force-pushed the nav-fetch-referrer branch 2 times, most recently from 1bc61b5 to 7726453 Compare April 13, 2019 00:36
@jdm
Copy link
Member

jdm commented Apr 13, 2019

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

⌛ Trying commit 7726453 with merge 2c879fb...

bors-servo pushed a commit that referenced this pull request Apr 13, 2019
Add referrer to navigation fetch request

<!-- Please describe your changes on the following line: -->
Implement step 13 of [following hyperlinks](https://html.spec.whatwg.org/#following-hyperlinks-2) and step 14.3 of [window open](https://html.spec.whatwg.org/#window-open-steps), as well as other referrer- and fetch-related updates.

---
<!-- 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 #22890 (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/23090)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Apr 13, 2019
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Apr 15, 2019
@miller-time
Copy link
Contributor Author

miller-time commented Apr 15, 2019

error: process didn't exit successfully: /home/travis/build/servo/servo/target/debug/deps/hang_monitor_tests-e2bf1aad70b8b79b (signal: 27)
The command "./mach test-unit" exited with 101.

Hmm I got this same error when running locally...

e: ran again with --verbose and it passed... then ran again without --verbose and it passed again... 🤔

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word "referrer" stopped making any sense to me while I was reading these changes, but this looks great to me! Thanks for clearing this up.

@jdm
Copy link
Member

jdm commented Apr 25, 2019

@bors-servo r=gterzian

@bors-servo
Copy link
Contributor

📌 Commit 2440e0f has been approved by gterzian

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Apr 25, 2019
@bors-servo
Copy link
Contributor

⌛ Testing commit 2440e0f with merge 8346513...

bors-servo pushed a commit that referenced this pull request Apr 25, 2019
Add referrer to navigation fetch request

<!-- Please describe your changes on the following line: -->
Implement step 13 of [following hyperlinks](https://html.spec.whatwg.org/#following-hyperlinks-2) and step 14.3 of [window open](https://html.spec.whatwg.org/#window-open-steps), as well as other referrer- and fetch-related updates.

---
<!-- 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 #22890 (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/23090)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 25, 2019
@jdm
Copy link
Member

jdm commented Apr 25, 2019

bors-servo pushed a commit that referenced this pull request Apr 25, 2019
Add referrer to navigation fetch request

<!-- Please describe your changes on the following line: -->
Implement step 13 of [following hyperlinks](https://html.spec.whatwg.org/#following-hyperlinks-2) and step 14.3 of [window open](https://html.spec.whatwg.org/#window-open-steps), as well as other referrer- and fetch-related updates.

---
<!-- 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 #22890 (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/23090)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

⌛ Testing commit 2440e0f with merge f89a163...

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 25, 2019
@bors-servo
Copy link
Contributor

💔 Test failed - status-taskcluster

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 25, 2019
@KiChjang
Copy link
Contributor

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 2440e0f with merge b73956c...

bors-servo pushed a commit that referenced this pull request Apr 26, 2019
Add referrer to navigation fetch request

<!-- Please describe your changes on the following line: -->
Implement step 13 of [following hyperlinks](https://html.spec.whatwg.org/#following-hyperlinks-2) and step 14.3 of [window open](https://html.spec.whatwg.org/#window-open-steps), as well as other referrer- and fetch-related updates.

---
<!-- 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 #22890 (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/23090)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 26, 2019
@bors-servo
Copy link
Contributor

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: gterzian
Pushing b73956c to master...

@bors-servo bors-servo merged commit 2440e0f into servo:master Apr 26, 2019
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 26, 2019
@gterzian
Copy link
Member

@miller-time thanks!

@miller-time
Copy link
Contributor Author

@gterzian my pleasure 🙂 and thanks to you and @jdm for all the guidance along the way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "referrer" to fetch request used in navigation
7 participants