Skip to content
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

Ensure transparent hit test region is sized and positioned correctly. #22839

Merged
merged 1 commit into from Feb 7, 2019

Conversation

@jdm
Copy link
Member

jdm commented Feb 6, 2019

This addresses some edge cases that were missed by #22156. Specifically, in some cases the transparent region was being placed in the display list in front of the scrollable content, rather than behind it, and the size of the region did not account for parent stacking context.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #22073 and fix #22216.
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Feb 6, 2019

Heads up! This PR modifies the following files:

  • @emilio: components/layout/display_list/builder.rs
@jdm
Copy link
Member Author

jdm commented Feb 6, 2019

To manually test these changes, play with scrolling in the leftmost box of http://asvd.github.io/dragscroll/.

r? @pcwalton

@highfive highfive assigned pcwalton and unassigned ferjm Feb 6, 2019
@pcwalton
Copy link
Contributor

pcwalton commented Feb 6, 2019

@bors-servo: r+

Looks good.

@bors-servo
Copy link
Contributor

bors-servo commented Feb 6, 2019

📌 Commit f59e95e has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Feb 6, 2019

Testing commit f59e95e with merge 5a21b7c...

bors-servo added a commit that referenced this pull request Feb 6, 2019
Ensure transparent hit test region is sized and positioned correctly.

This addresses some edge cases that were missed by #22156. Specifically, in some cases the transparent region was being placed in the display list in front of the scrollable content, rather than behind it, and the size of the region did not account for parent stacking context.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22073 and fix #22216.
- [x] There are tests for these changes

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

bors-servo commented Feb 6, 2019

💔 Test failed - mac-rel-css1

@jdm
Copy link
Member Author

jdm commented Feb 6, 2019

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Feb 6, 2019

Testing commit f59e95e with merge d81ff57...

bors-servo added a commit that referenced this pull request Feb 6, 2019
Ensure transparent hit test region is sized and positioned correctly.

This addresses some edge cases that were missed by #22156. Specifically, in some cases the transparent region was being placed in the display list in front of the scrollable content, rather than behind it, and the size of the region did not account for parent stacking context.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22073 and fix #22216.
- [x] There are tests for these changes

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

bors-servo commented Feb 7, 2019

@bors-servo bors-servo merged commit f59e95e into servo:master Feb 7, 2019
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@jdm jdm deleted the jdm:overflow-hit-test-fix branch Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.