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.

Sign up for GitHub

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

Jump to bottom

overflow:hidden changes the stacking order #8122

Closed
paulrouget opened this issue Oct 21, 2015 · 11 comments
Closed

overflow:hidden changes the stacking order #8122

paulrouget opened this issue Oct 21, 2015 · 11 comments
Labels
A-gfx/displaylist C-assigned There is someone working on resolving the issue

Comments

@paulrouget
Copy link
Contributor 

<style>

  div {
    width: 100px;
    height: 100px;
  }

  #a {
    background: red;
    position: absolute;
    top: 10px;
    left: 10px;
    overflow: hidden;
  }

  #b {
    background: blue;
    position: absolute;
    top: 20px;
    left: 20px;
  }
</style>

<body>
  <div id='a'></div>
  <div id='b'></div>
</body>

screen shot 2015-10-21 at 11 58 11
@paulrouget paulrouget mentioned this issue Oct 21, 2015
Closed
92 tasks
@gilles-leblanc
Copy link
Contributor

If this one is free for the taking I would like to have a look into it.

@jdm
Copy link
Member

jdm commented Oct 23, 2015

Sure looks like it!

@jdm jdm added the C-assigned There is someone working on resolving the issue label Oct 23, 2015
@gilles-leblanc
Copy link
Contributor

For the moment I'm stuck with this one.

I have established that it works with overflow auto, scroll and visible.

I have looked in components/gfx/display_list.

There's two files in this directory. One of them is the optimizer.

I have taken out the optimizer (I've had it return the same display_list it was given in input as it's output) and the problem is still present, so that rules out an optimizer bug.

I then looked at the mod.rs file. I then also looked at components/layout/[block, display_list_builder].rs. I don't understand everything going on in all those files yet though. Display lists, fragments, stacking order and even block elements are all pretty new to me so that makes a lot to absorb.

I've search components/gfx and components/layout for mentions of the terms overflow, hidden and absolute positioning and looked at that code and made some changes to see if they had any impact.

Do you have any suggestions as to where I could look next or any ideas?

I'm hoping for some angle on how to tackle this next.

@eefriedman
Copy link
Contributor

CC @mrobinson

@jdm jdm added the C-looking for assistance The person attempting to resolve the issue requires further help label Oct 31, 2015
@metajack
Copy link
Contributor

metajack commented Nov 2, 2015

cc @pcwalton @glennw

This is blocking browser.html work.

@mrobinson
Copy link
Member

I believe this is also fixed by #8266.

mrobinson added a commit to mrobinson/servo that referenced this issue Nov 2, 2015
@mrobinson
Mix stacking contexts into the positioned content list
90a16a4 
Sometimes positioned content needs to be layered on top of stacking
contexts. The layer synthesis code can do this, but the current design
prevents it because stacking contexts are stored in a separate struct
member. In order to preserve tree order, mix stacking contexts into the
positioned content list, by adding a new StackingContextClass
DisplayItem. Such items do not have a base DisplayItem.

In some ways this simplifies the code, because we no longer have to
have a separate code path in the StackingContextLayerCreator.

Fixes servo#7779.
Fixes servo#7983.
Fixes servo#8122.
mrobinson added a commit to mrobinson/servo that referenced this issue Nov 2, 2015
@mrobinson
Mix stacking contexts into the positioned content list
a2acb2a 
Sometimes positioned content needs to be layered on top of stacking
contexts. The layer synthesis code can do this, but the current design
prevents it because stacking contexts are stored in a separate struct
member. In order to preserve tree order, mix stacking contexts into the
positioned content list, by adding a new StackingContextClass
DisplayItem. Such items do not have a base DisplayItem.

In some ways this simplifies the code, because we no longer have to
have a separate code path in the StackingContextLayerCreator.

Fixes servo#7779.
Fixes servo#7983.
Fixes servo#8122.
gilles-leblanc added a commit to gilles-leblanc/servo that referenced this issue Nov 3, 2015
@gilles-leblanc
Adds wpt css test for stacking order bug
b2ba72a 
Adding overflow:hidden to an absolute positioned div will change it's
stacking order.

referencing servo#8122
@gilles-leblanc
Copy link
Contributor

@mrobinson I have made a wpt test with the case written by @paulrouget which I have committed to a branch (gilles-leblanc@b2ba72a). I then pulled and merged your branch master...mrobinson:stacking-context-mix and tested it but I'm still getting a failure on the test case.

@mrobinson
Copy link
Member

@gilles-leblanc I ran your test locally on top of master...mrobinson:stacking-context-mix and it seems to pass for me. Can you double-check by pulling the branch at https://github.com/mrobinson/servo/tree/gilles-test and testing again? If it continues to fail for you, would you post or describe the image diff from the test results analyzer at http://hoppipolla.co.uk/410/reftest-analyser-structured.xhtml?

@gilles-leblanc
Copy link
Contributor

@mrobinson I pulled the branch and it's still failing but it's probably something on my side, some sort of mistake I did. I've created a gist with the log file: https://gist.github.com/gilles-leblanc/8e6467040d342272a996 which you can paste in the reftest analyser.

@mrobinson
Copy link
Member

@gilles-leblanc Tests that were added for issues in previous PRs that I have landed seem to be failing as well. Can you double-check that you are not running the tests with a stale build? One thing that I sometimes do is to only compile a debug build and accidentally run the tests with --release flag or the opposite.

@gilles-leblanc
Copy link
Contributor

@mrobinson Oh my bad I thought that the tests were ran with the last build, in my case a release build so I didn't supply the --release switch for the test command.

Sorry!

Everything now works.

@jdm jdm removed the C-looking for assistance The person attempting to resolve the issue requires further help label Nov 3, 2015
mrobinson added a commit to mrobinson/servo that referenced this issue Nov 3, 2015
@mrobinson
Mix stacking contexts into the positioned content list
5b3e953 
Sometimes positioned content needs to be layered on top of stacking
contexts. The layer synthesis code can do this, but the current design
prevents it because stacking contexts are stored in a separate struct
member. In order to preserve tree order, mix stacking contexts into the
positioned content list, by adding a new StackingContextClass
DisplayItem. Such items do not have a base DisplayItem.

In some ways this simplifies the code, because we no longer have to
have a separate code path in the StackingContextLayerCreator.

Fixes servo#7779.
Fixes servo#7983.
Fixes servo#8122.
Fixes servo#8310.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-gfx/displaylist C-assigned There is someone working on resolving the issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mrobinson @jdm @metajack @paulrouget @gilles-leblanc @eefriedman