From f68386f0fa9dfe79deb8a73f45a93afe40f2561d Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 25 Feb 2015 13:46:02 +1000 Subject: [PATCH] Fix making an iframe visible when it was initially set to display:none. When an iframe is created with display:none it sets the root layer to be zero width and height. When updating the rect of the iframe from layout send the entire rect rather than just the new origin, which handles the case where the iframe has been made visible and now has a non-zero rect. --- components/compositing/compositor.rs | 16 ++++++++-------- components/compositing/compositor_task.rs | 6 +++--- components/compositing/constellation.rs | 4 ++-- components/compositing/headless.rs | 2 +- tests/ref/basic.list | 1 + tests/ref/iframe/hide_and_show.html | 20 ++++++++++++++++++++ tests/ref/iframe/hide_and_show_ref.html | 13 +++++++++++++ 7 files changed, 48 insertions(+), 14 deletions(-) create mode 100644 tests/ref/iframe/hide_and_show.html create mode 100644 tests/ref/iframe/hide_and_show_ref.html diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 3ed89b37604..bf5370c4a92 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -315,9 +315,9 @@ impl IOCompositor { chan.send(Some(self.window.native_metadata())).unwrap(); } - (Msg::SetLayerOrigin(pipeline_id, layer_id, origin), + (Msg::SetLayerRect(pipeline_id, layer_id, rect), ShutdownState::NotShuttingDown) => { - self.set_layer_origin(pipeline_id, layer_id, origin); + self.set_layer_rect(pipeline_id, layer_id, &rect); } (Msg::AssignPaintedBuffers(pipeline_id, epoch, replies), ShutdownState::NotShuttingDown) => { @@ -721,15 +721,15 @@ impl IOCompositor { self.composition_request = CompositionRequest::CompositeOnScrollTimeout(timestamp); } - fn set_layer_origin(&mut self, - pipeline_id: PipelineId, - layer_id: LayerId, - new_origin: Point2D) { + fn set_layer_rect(&mut self, + pipeline_id: PipelineId, + layer_id: LayerId, + new_rect: &Rect) { match self.find_layer_with_pipeline_and_layer_id(pipeline_id, layer_id) { Some(ref layer) => { - layer.bounds.borrow_mut().origin = Point2D::from_untyped(&new_origin) + *layer.bounds.borrow_mut() = Rect::from_untyped(new_rect) } - None => panic!("Compositor received SetLayerOrigin for nonexistent \ + None => panic!("Compositor received SetLayerRect for nonexistent \ layer: {:?}", pipeline_id), }; diff --git a/components/compositing/compositor_task.rs b/components/compositing/compositor_task.rs index f3504aa6704..e3c1a5102d1 100644 --- a/components/compositing/compositor_task.rs +++ b/components/compositing/compositor_task.rs @@ -188,8 +188,8 @@ pub enum Msg { /// Tells the compositor to create a descendant layer for a pipeline if necessary (i.e. if no /// layer with that ID exists). CreateOrUpdateDescendantLayer(LayerProperties), - /// Alerts the compositor that the specified layer's origin has changed. - SetLayerOrigin(PipelineId, LayerId, Point2D), + /// Alerts the compositor that the specified layer's rect has changed. + SetLayerRect(PipelineId, LayerId, Rect), /// Scroll a page in a window ScrollFragmentPoint(PipelineId, LayerId, Point2D), /// Requests that the compositor assign the painted buffers to the given layers. @@ -231,7 +231,7 @@ impl Debug for Msg { Msg::GetGraphicsMetadata(..) => write!(f, "GetGraphicsMetadata"), Msg::CreateOrUpdateBaseLayer(..) => write!(f, "CreateOrUpdateBaseLayer"), Msg::CreateOrUpdateDescendantLayer(..) => write!(f, "CreateOrUpdateDescendantLayer"), - Msg::SetLayerOrigin(..) => write!(f, "SetLayerOrigin"), + Msg::SetLayerRect(..) => write!(f, "SetLayerRect"), Msg::ScrollFragmentPoint(..) => write!(f, "ScrollFragmentPoint"), Msg::AssignPaintedBuffers(..) => write!(f, "AssignPaintedBuffers"), Msg::ChangeReadyState(..) => write!(f, "ChangeReadyState"), diff --git a/components/compositing/constellation.rs b/components/compositing/constellation.rs index daf2cb2503d..2d370eca4e5 100644 --- a/components/compositing/constellation.rs +++ b/components/compositing/constellation.rs @@ -681,10 +681,10 @@ impl Constellation { initial_viewport: rect.size * ScaleFactor(1.0), device_pixel_ratio: device_pixel_ratio, })).unwrap(); - compositor_proxy.send(CompositorMsg::SetLayerOrigin( + compositor_proxy.send(CompositorMsg::SetLayerRect( pipeline.id, LayerId::null(), - rect.to_untyped().origin)); + rect.to_untyped())); } else { already_sent.insert(pipeline.id); } diff --git a/components/compositing/headless.rs b/components/compositing/headless.rs index 3c57b0e3b82..0c38846d1d0 100644 --- a/components/compositing/headless.rs +++ b/components/compositing/headless.rs @@ -104,7 +104,7 @@ impl CompositorEventListener for NullCompositor { Msg::CreateOrUpdateBaseLayer(..) | Msg::CreateOrUpdateDescendantLayer(..) | - Msg::SetLayerOrigin(..) | + Msg::SetLayerRect(..) | Msg::AssignPaintedBuffers(..) | Msg::ChangeReadyState(..) | Msg::ChangePaintState(..) | diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 67dfebfdd6d..e8e2ecfb306 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -137,6 +137,7 @@ fragment=top != ../html/acid2.html acid2_ref.html == iframe/multiple_external.html iframe/multiple_external_ref.html == iframe/overflow.html iframe/overflow_ref.html == iframe/positioning_margin.html iframe/positioning_margin_ref.html +== iframe/hide_and_show.html iframe/hide_and_show_ref.html == floated_generated_content_a.html floated_generated_content_b.html == inline_block_margin_a.html inline_block_margin_ref.html diff --git a/tests/ref/iframe/hide_and_show.html b/tests/ref/iframe/hide_and_show.html new file mode 100644 index 00000000000..29ab83dfe2a --- /dev/null +++ b/tests/ref/iframe/hide_and_show.html @@ -0,0 +1,20 @@ + + + + + + + + diff --git a/tests/ref/iframe/hide_and_show_ref.html b/tests/ref/iframe/hide_and_show_ref.html new file mode 100644 index 00000000000..1580b58858c --- /dev/null +++ b/tests/ref/iframe/hide_and_show_ref.html @@ -0,0 +1,13 @@ + + + + +
+ +