1008787 - Plugins should be clipped to the size layout expects them to be

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Clip plugins to their expected content size

Plugins should be clipped to the size we expect their image to be. If we don't do this then a plugin with it's size asynchronously updating could display outside the bounds where layout expects it to be.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Clip plugins to their expected content size v2

Updated to make GetLeafLayerFor work as advertised.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Clip plugins to their expected content size v3

Correct usage of nsIntRect::Intersect().

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8420747 [details] [diff] [review]
Clip plugins to their expected content size v3

Review of attachment 8420747 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsObjectFrame.cpp
@@ +1617,5 @@
>    layer->SetBaseTransform(Matrix4x4::From2D(transform));
>    layer->SetVisibleRegion(ThebesIntRect(IntRect(IntPoint(0, 0), size)));
> +
> +  nsIntRect clipRect(p.x, p.y, size.width, size.height);
> +  layer->SetClipRect(&clipRect);

It would be cleaner to just put this clip rect on the display item, then we wouldn't have to change FrameLayerBuilder. I'm pretty sure that's doable in nsObjectFrame::BuildDisplayList. Maybe we just need to remove DisplayListClipState::ASSUME_DRAWING_RESTRICTED_TO_CONTENT_RECT from DisplayListClipState::AutoClipContainingBlockDescendantsToContentBox there?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8420747 [details] [diff] [review]
Clip plugins to their expected content size v3

Review of attachment 8420747 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, didn't mean to r+ that

Comment 5

[Bas Schouten (:bas.schouten)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> Comment on attachment 8420747 [details] [diff] [review]
> Clip plugins to their expected content size v3
> 
> Review of attachment 8420747 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsObjectFrame.cpp
> @@ +1617,5 @@
> >    layer->SetBaseTransform(Matrix4x4::From2D(transform));
> >    layer->SetVisibleRegion(ThebesIntRect(IntRect(IntPoint(0, 0), size)));
> > +
> > +  nsIntRect clipRect(p.x, p.y, size.width, size.height);
> > +  layer->SetClipRect(&clipRect);
> 
> It would be cleaner to just put this clip rect on the display item, then we
> wouldn't have to change FrameLayerBuilder. I'm pretty sure that's doable in
> nsObjectFrame::BuildDisplayList. Maybe we just need to remove
> DisplayListClipState::ASSUME_DRAWING_RESTRICTED_TO_CONTENT_RECT from
> DisplayListClipState::AutoClipContainingBlockDescendantsToContentBox there?

Could be.. I'm all in favor, I just don't know any of this code so I merely did whatever it took to fix the bug :-). Fwiw, I found out GetOldLayer didn't clear the clipRect either so that at least would need fixing.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Clip plugins

I can't reproduce the failure locally, but confirmed that this clips the ImageLayer to the expected size of the plugin.

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d37218a619e

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2d37218a619e