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

Jump to bottom

Iframe content not painted at the right coordinate if the iframe is in a transformed element #7377

Closed
paulrouget opened this issue Aug 26, 2015 · 3 comments
Closed

Iframe content not painted at the right coordinate if the iframe is in a transformed element #7377

paulrouget opened this issue Aug 26, 2015 · 3 comments
Labels
A-layout/inline

Comments

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Aug 26, 2015

Related: #7282

<body style="background:red">
  <div style="transform: scale(1)">
    <iframe src="http://example.com"></iframe>
  </div>
</body>

screen shot 2015-08-26 at 10 51 15
@paulrouget paulrouget mentioned this issue Aug 26, 2015
Closed
62 of 92 tasks complete
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Aug 26, 2015

cc @pcwalton
@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Aug 26, 2015

Iiiiiiinteresting. Would have thought that would be fixed by #7282.
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 28, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
d4d87ed 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes servo#7377.
@pcwalton pcwalton mentioned this issue Aug 28, 2015
Merged
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 28, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
791ec95 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes servo#7377.
bors-servo pushed a commit that referenced this issue Aug 28, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
0356328 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 28, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
a6737e4 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes servo#7377.
bors-servo pushed a commit that referenced this issue Aug 28, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
8e5f862 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 28, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
e2c44bf 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 31, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
2a9d0bc 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
pcwalton added a commit to pcwalton/servo that referenced this issue Aug 31, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
37a69f5 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
bors-servo pushed a commit that referenced this issue Aug 31, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
ec5f565 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
pcwalton added a commit to pcwalton/servo that referenced this issue Sep 24, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
95c434e 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
pcwalton added a commit to pcwalton/servo that referenced this issue Sep 24, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
e22c92f 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
pcwalton added a commit to pcwalton/servo that referenced this issue Sep 24, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
a3acd42 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
pcwalton added a commit to pcwalton/servo that referenced this issue Sep 27, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
002e120 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
bors-servo pushed a commit that referenced this issue Sep 27, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
da904a1 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Sep 29, 2015

I will add to this that also the z-index is wrong:

<style>

  body {
    background: yellow;
  }

  div, iframe {
    position: absolute;
    width: 200px;
    height: 200px;
  }

  iframe {
    border: 10px solid black;
    top: 0px;
    left: 0px;
  }

  div {
    left: 100px;
    top: 100px;
    background: red;
  }

</style>

<body>

  <iframe src='http://example.com'></iframe>
  <div> </div>

</body>

screen shot 2015-09-29 at 10 43 26
@jdm jdm added the A-layout/inline label Sep 29, 2015
pcwalton added a commit to pcwalton/servo that referenced this issue Sep 29, 2015
@pcwalton
layout: Make the compositor rather than layout determine the position of
c72d0c2 
each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that both the sizes and positions of
iframes are determined by the compositor. Layout layerizes all iframes
and marks the iframe layers with the appropriate pipeline and subpage
IDs so that the compositor can place them correctly. This approach is
similar in spirit to Gecko's `RefLayer` infrastructure. The logic that
determines when it is time to take the screenshot for reftests has been
significantly revamped to deal with this change in delegation of
responsibility.

Additionally, this code removes the infrastructure that sends layout
data back to the layout task to be destroyed, since it is now all
thread-safe and can be destroyed on the script task.

The failing tests now fail because of a pre-existing bug related to
intrinsic heights and borders on inline replaced elements. They happened
to pass before because we never rendered the iframes at all, which meant
they never had a chance to draw the red border the tests expect to not
render!

Closes servo#7377.
bors-servo pushed a commit that referenced this issue Sep 30, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
29c061e 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Sep 30, 2015
bors-servo
Auto merge of #7423 - pcwalton:iframe-stacking-context-position, r=gl…
Loading status checks…
a0cb657 
…ennw

layout: Make the compositor rather than layout determine the position of each iframe.

The old code that attempted to do this during layout wasn't able to work
for multiple reasons: it couldn't know where the iframe was going to be
on the page (because of nested iframes), and at the time it was building
the display list for a fragment it couldn't know where that fragment was
going to be in page coordinates.

This patch rewrites that code so that only the size of an iframe is
determined during layout, and the position is determined by the
compositor. Layout layerizes iframes and marks the iframe layers with
the appropriate subpage ID so that the compositor can place them
correctly.

Closes #7377.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7423)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-layout/inline
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@jdm @pcwalton @paulrouget
You can’t perform that action at this time.