Merge lp:~alan-griffiths/miral/fix-1631958 into lp:miral

[Alan Griffiths]

Invisible windows are poor candidates for receiving focus. Especially if they are modal dialogs (LP:1631958)

[Gerry Boland (gerboland)]

Yep, looks good & tests ok, thanks

review: Approve

=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2016-10-05 09:07:53 +0000
+++ miral/basic_window_manager.cpp	2016-10-10 14:59:30 +0000
@@ -843,6 +843,8 @@
     policy->advise_state_change(window_info, value);
     window_info.state(value);
 
+    mir_surface->configure(mir_surface_attrib_state, value);
+
     switch (value)
     {
     case mir_surface_state_hidden:
@@ -856,6 +858,8 @@
                 {
                     if (candidate == window)
                         return true;
+                    if (!std::shared_ptr<scene::Surface>(candidate)->visible())
+                        return true;
                     auto const w = candidate;
                     return !(select_active_window(w));
                 });
@@ -865,8 +869,6 @@
     default:
         mir_surface->show();
     }
-
-    mir_surface->configure(mir_surface_attrib_state, value);
 }
 
 void miral::BasicWindowManager::update_event_timestamp(MirKeyboardEvent const* kev)
@@ -930,8 +932,8 @@
     {
         if (std::shared_ptr<mir::scene::Surface> surface = child)
         {
-            if (surface->type() == mir_surface_type_dialog)
-                return (select_active_window(child));
+            if (surface->type() == mir_surface_type_dialog && surface->visible())
+                return select_active_window(child);
         }
     }
 

=== modified file 'test/select_active_window.cpp'
--- test/select_active_window.cpp	2016-09-23 11:47:24 +0000
+++ test/select_active_window.cpp	2016-10-10 14:59:30 +0000
@@ -96,4 +96,28 @@
     auto actual = basic_window_manager.select_active_window(parent);
     EXPECT_THAT(actual, Eq(dialog))
         << "actual=" << actual << ", expected=" << dialog;
+}
+
+TEST_F(SelectActiveWindow, given_a_hidden_child_dialog_when_selecting_the_parent_the_parent_receives_focus)
+{
+    mir::scene::SurfaceCreationParameters creation_parameters;
+    creation_parameters.name = "parent";
+    creation_parameters.type = mir_surface_type_normal;
+    creation_parameters.size = Size{600, 400};
+
+    auto parent = create_window(creation_parameters);
+
+    creation_parameters.name = "dialog";
+    creation_parameters.type = mir_surface_type_dialog;
+    creation_parameters.parent = parent;
+
+    auto dialog = create_window(creation_parameters);
+
+    WindowSpecification mods;
+    mods.state() = mir_surface_state_hidden;
+    basic_window_manager.modify_window(basic_window_manager.info_for(dialog), mods);
+
+    auto actual = basic_window_manager.select_active_window(parent);
+    EXPECT_THAT(actual, Eq(parent))
+                << "actual=" << actual << ", expected=" << parent;
 }
\ No newline at end of file

=== modified file 'test/test_window_manager_tools.h'
--- test/test_window_manager_tools.h	2016-09-30 09:39:48 +0000
+++ test/test_window_manager_tools.h	2016-10-10 14:59:30 +0000
@@ -86,12 +86,25 @@
     mir::geometry::Size size() const override { return  size_; }
     void resize(mir::geometry::Size const& size) override { size_ = size; }
 
-    bool visible() const override { return  true; }
+    auto state() const -> MirSurfaceState override { return state_; }
+    auto configure(MirSurfaceAttrib attrib, int value) -> int override {
+        switch (attrib)
+        {
+        case mir_surface_attrib_state:
+            state_ = MirSurfaceState(value);
+            return state_;
+        default:
+            return value;
+        }
+    }
+
+    bool visible() const override { return  state() != mir_surface_state_hidden; }
 
     std::string name_;
     MirSurfaceType type_;
     mir::geometry::Point top_left_;
     mir::geometry::Size size_;
+    MirSurfaceState state_ = mir_surface_state_restored;
 };
 
 struct StubStubSession : mir::test::doubles::StubSession