Skip to content

/ servo

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

Use reference frames explicitly for fixed positioning #20767

Merged
merged 1 commit into from Jun 5, 2018
Merged

Use reference frames explicitly for fixed positioning #20767

merged 1 commit into from Jun 5, 2018

Conversation

@mrobinson
Copy link
Member

mrobinson commented May 9, 2018 •
edited by SimonSapin

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes because they should not change behavior.
  • These changes do not require tests because _____

This change is Reviewable
@highfive
Copy link

highfive commented May 9, 2018

Heads up! This PR modifies the following files:

  • @paulrouget: components/compositing/compositor.rs
  • @emilio: components/layout/display_list/items.rs, components/layout/display_list/builder.rs, components/layout/fragment.rs, components/layout/display_list/webrender_helpers.rs
@highfive
Copy link

highfive commented May 9, 2018

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@mrobinson
Copy link
Member Author

mrobinson commented May 9, 2018

r? @glennw or anyone
@mrobinson mrobinson changed the title Use reference frame for fixed position Use reference frames explicitly for fixed positioning May 9, 2018
@mrobinson mrobinson force-pushed the mrobinson:use-reference-frame-for-fixed-position branch from f69eeb7 to 21673b4 May 9, 2018
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2018

☔ The latest upstream changes (presumably #20752) made this pull request unmergeable. Please resolve the merge conflicts.
@mrobinson mrobinson force-pushed the mrobinson:use-reference-frame-for-fixed-position branch from 21673b4 to bfdb977 May 30, 2018
@mrobinson mrobinson mentioned this pull request May 30, 2018
Merged
@gw3583
Copy link
Contributor

gw3583 commented Jun 1, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 1, 2018

📌 Commit bfdb977 has been approved by gw3583
@highfive highfive assigned gw3583 and unassigned glennw Jun 1, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jun 1, 2018

⌛ Testing commit bfdb977 with merge a88595f...
bors-servo added a commit that referenced this pull request Jun 1, 2018
@bors-servo
Auto merge of #20767 - mrobinson:use-reference-frame-for-fixed-positi…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a88595f 
…on, r=gw3583

Use reference frames explicitly for fixed positioning

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes because they should not change behavior.
- [ ] 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/20767)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 1, 2018

💔 Test failed - linux-rel-css
@gw3583
Copy link
Contributor

gw3583 commented Jun 1, 2018 

{"status": "FAIL", "group": "default", "message": "assert_equals: expected Element node <html><head><meta charset=\"utf-8\">\n<title>Ensure that the... but got Element node <a id=\"link\" href=\"http://mozilla.org\">This is some link ...", "stack": "@http://web-platform.test:8000/_mozilla/mozilla/scrollBy.html:23:9\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1538:20\ntest@http://web-platform.test:8000/resources/testharness.js:548:9\n@http://web-platform.test:8000/_mozilla/mozilla/scrollBy.html:10:5\n", "subtest": "Ensure that the window.scrollBy function affects scroll position as expected", "test": "/_mozilla/mozilla/scrollBy.html", "line": 123654, "action": "test_result", "expected": "PASS"}

Not sure if this is an intermittent, but it seems like it's related to the changes in this patch.
@mrobinson mrobinson force-pushed the mrobinson:use-reference-frame-for-fixed-position branch from bfdb977 to 506b53b Jun 1, 2018
@mrobinson
Copy link
Member Author

mrobinson commented Jun 1, 2018

@gw3583 Looks like the issue was that I was assigning nodes to the root reference frame instead of the root scroll node. I've fixed this and made changes to ensure that this mistake is much harder to make. Here is the interdiff for the new version of the branch: https://gist.github.com/mrobinson/ea4879f787faeef0306c0f1202752a65
@mrobinson mrobinson mentioned this pull request Jun 4, 2018
Merged
@mrobinson
Copy link
Member Author

mrobinson commented Jun 4, 2018

@bors-servo try

The bot failure looks like an unrelated network failure.
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

⌛ Trying commit 32f00ef with merge ac39b4e...
bors-servo added a commit that referenced this pull request Jun 4, 2018
@bors-servo
Auto merge of #20767 - mrobinson:use-reference-frame-for-fixed-positi…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ac39b4e 
…on, r=<try>

Use reference frames explicitly for fixed positioning

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes because they should not change behavior.
- [ ] 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/20767)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

💔 Test failed - mac-rel-css1
@mrobinson
Copy link
Member Author

mrobinson commented Jun 4, 2018

The test failure is /fetch/range/sw.https.window.html. I'm fairly certain this is not related to my change. @gw3583 Sorry for the delay here, but this should be ready to review.
@gw3583
Copy link
Contributor

gw3583 commented Jun 4, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

📌 Commit 32f00ef has been approved by gw3583
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

⌛ Testing commit 32f00ef with merge 5040211...
bors-servo added a commit that referenced this pull request Jun 4, 2018
@bors-servo
Auto merge of #20767 - mrobinson:use-reference-frame-for-fixed-positi…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5040211 
…on, r=gw3583

Use reference frames explicitly for fixed positioning

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes because they should not change behavior.
- [ ] 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/20767)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2018

💔 Test failed - mac-rel-wpt3
@mrobinson
Copy link
Member Author

mrobinson commented Jun 5, 2018

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2018

⌛ Testing commit 32f00ef with merge a07c718...
bors-servo added a commit that referenced this pull request Jun 5, 2018
@bors-servo
Auto merge of #20767 - mrobinson:use-reference-frame-for-fixed-positi…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a07c718 
…on, r=gw3583

Use reference frames explicitly for fixed positioning

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes because they should not change behavior.
- [ ] 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/20767)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2018

💔 Test failed - mac-rel-css1
@mrobinson
Copy link
Member Author

mrobinson commented Jun 5, 2018

I don't really see a bug that matches these intermittent WebSocket failures (apart from perhaps #20734, but without the initial error). I'm going to retry this and then open a bug if it does manage to land.
@mrobinson
Copy link
Member Author

mrobinson commented Jun 5, 2018

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2018

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

bors-servo commented Jun 5, 2018

☀️ Test successful - android, 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: gw3583
Pushing a07c718 to master...
@bors-servo bors-servo merged commit 32f00ef into servo:master Jun 5, 2018
3 checks passed
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
@mrobinson mrobinson deleted the mrobinson:use-reference-frame-for-fixed-position branch Jun 5, 2018
@mrobinson mrobinson mentioned this pull request Jun 5, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gw3583 gw3583

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@mrobinson @highfive @bors-servo @gw3583 @glennw
You can’t perform that action at this time.