Merge lp:~alan-griffiths/miral/fix-focus-and-movement-by-window-type into lp:miral

Proposed by Alan Griffiths
Status: Merged
Approved by: Gerry Boland
Approved revision: 508
Merged at revision: 501
Proposed branch: lp:~alan-griffiths/miral/fix-focus-and-movement-by-window-type
Merge into: lp:miral
Prerequisite: lp:~alan-griffiths/miral/more-surface-to-window-renaming
Diff against target: 412 lines (+179/-40)
16 files modified
CMakeLists.txt (+1/-1)
debian/changelog (+10/-3)
debian/libmiral2.symbols (+3/-1)
include/miral/toolkit/window_spec.h (+42/-6)
include/miral/window_manager_tools.h (+3/-0)
miral/basic_window_manager.cpp (+5/-0)
miral/basic_window_manager.h (+2/-0)
miral/symbols.map (+7/-0)
miral/window_info.cpp (+4/-4)
miral/window_management_trace.cpp (+11/-0)
miral/window_management_trace.h (+2/-0)
miral/window_manager_tools.cpp (+3/-0)
miral/window_manager_tools_implementation.h (+1/-0)
scripts/process_doxygen_xml.py (+9/-1)
test/active_window.cpp (+38/-0)
test/drag_active_window.cpp (+38/-24)
To merge this branch: bzr merge lp:~alan-griffiths/miral/fix-focus-and-movement-by-window-type
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Review via email: mp+316255@code.launchpad.net

Commit message

[libmiral] Fix focus and movement rules for Input Method and Satellite windows

Description of the change

A careful reading of the spec[1] says that Input Method windows and Satellite windows should not get focus.

In fixing this it became apparent that WindowManagementTools only supported dragging of the active window. But Satellite windows are movable - so we needed to fix that too.

That means we introduced a new ABI, so we also start on release 1.2 with the corresponding stanza.

[1] "An input aid is a surface that has permission to deliver keyboard or pointer input to the surface that has input focus. ... In all other respects, an input aid should behave the same as a satellite that is constantly reparented to whichever window has input focus."

To post a comment you must log in.
Revision history for this message
Gerry Boland (gerboland) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2017-01-20 11:35:32 +0000
3+++ CMakeLists.txt 2017-02-02 18:03:45 +0000
4@@ -41,7 +41,7 @@
5 include_directories(include SYSTEM ${MIRCLIENT_INCLUDE_DIRS})
6
7 set(MIRAL_VERSION_MAJOR 1)
8-set(MIRAL_VERSION_MINOR 1)
9+set(MIRAL_VERSION_MINOR 2)
10 set(MIRAL_VERSION_PATCH 0)
11
12 set(MIRAL_VERSION ${MIRAL_VERSION_MAJOR}.${MIRAL_VERSION_MINOR}.${MIRAL_VERSION_PATCH})
13
14=== modified file 'debian/changelog'
15--- debian/changelog 2017-01-26 11:36:17 +0000
16+++ debian/changelog 2017-02-02 18:03:45 +0000
17@@ -1,5 +1,12 @@
18-miral (1.1.0) UNRELEASED; urgency=medium
19-
20+miral (1.2.0) UNRELEASED; urgency=medium
21+
22+ *
23+
24+ -- Alan Griffiths <alan.griffiths@canonical.com> Thu, 02 Feb 2017 17:26:54 +0000
25+
26+miral (1.1.0+17.04.20170127-0ubuntu1) zesty; urgency=medium
27+
28+ [ Alan Griffiths ]
29 * New upstream release 1.1.0 (https://launchpad.net/miral/+milestone/1.1)
30 - ABI summary:
31 . miral ABI unchanged at 2
32@@ -13,7 +20,7 @@
33 . miral-shell depends on default cursor theme being installed
34 (LP: #1658159)
35
36- -- Alan Griffiths <alan.griffiths@canonical.com> Wed, 14 Dec 2016 16:31:08 +0000
37+ -- Cemil Azizoglu <cemil.azizoglu@canonical.com> Fri, 27 Jan 2017 03:02:28 +0000
38
39 miral (1.0.2+17.04.20170118.1-0ubuntu1) zesty; urgency=medium
40
41
42=== modified file 'debian/libmiral2.symbols'
43--- debian/libmiral2.symbols 2017-01-26 11:30:51 +0000
44+++ debian/libmiral2.symbols 2017-02-02 18:03:45 +0000
45@@ -361,4 +361,6 @@
46 (c++)"miral::WindowInfo::state(MirWindowState)@MIRAL_1.1" 1.1.0
47 (c++)"miral::WindowManagementPolicy::advise_state_change(miral::WindowInfo const&, MirWindowState)@MIRAL_1.1" 1.1.0
48 (c++)"miral::WindowInfo::can_morph_to(MirWindowType) const@MIRAL_1.1" 1.1.0
49- (c++)"miral::CanonicalWindowManagerPolicy::place_new_window(miral::ApplicationInfo const&, miral::WindowSpecification const&)@MIRAL_1.1" 1.1.0
50\ No newline at end of file
51+ (c++)"miral::CanonicalWindowManagerPolicy::place_new_window(miral::ApplicationInfo const&, miral::WindowSpecification const&)@MIRAL_1.1" 1.1.0
52+ MIRAL_1.2@MIRAL_1.2 1.2.0
53+ (c++)"miral::WindowManagerTools::drag_window(miral::Window const&, mir::geometry::Displacement)@MIRAL_1.2" 1.2.0
54
55=== modified file 'include/miral/toolkit/window_spec.h'
56--- include/miral/toolkit/window_spec.h 2017-02-02 18:03:45 +0000
57+++ include/miral/toolkit/window_spec.h 2017-02-02 18:03:45 +0000
58@@ -58,12 +58,6 @@
59 {
60 return WindowSpec{mir_create_normal_window_spec(connection, width, height)};
61 }
62-
63- auto set_pixel_format(MirPixelFormat format) -> WindowSpec&
64- {
65- mir_window_spec_set_pixel_format(*this, format);
66- return *this;
67- }
68 #endif
69
70 static auto for_menu(MirConnection* connection,
71@@ -125,6 +119,38 @@
72 return for_dialog(connection, width, height, format).set_parent(parent);
73 }
74
75+ static auto for_input_method(MirConnection* connection, int width, int height, MirWindow* parent)
76+ {
77+#if MIR_CLIENT_VERSION > MIR_VERSION_NUMBER(3, 4, 0)
78+ auto spec = WindowSpec{mir_create_input_method_window_spec(connection, width, height)}
79+#else
80+ auto spec = WindowSpec{mir_create_surface_spec(connection)}
81+ .set_buffer_usage(mir_buffer_usage_hardware) // Required protobuf field for create_window()
82+ .set_pixel_format(mir_pixel_format_invalid) // Required protobuf field for create_window()
83+ .set_size(width, height)
84+ .set_type(mir_window_type_inputmethod)
85+#endif
86+ .set_parent(parent);
87+ return spec;
88+ }
89+
90+ static auto for_satellite(MirConnection* connection, int width, int height, MirWindow* parent)
91+ {
92+#if MIR_CLIENT_VERSION > MIR_VERSION_NUMBER(3, 4, 0)
93+ // There's no mir_create_satellite_window_spec()
94+ auto spec = WindowSpec{mir_create_window_spec(connection)}
95+#else
96+ // There's no mir_create_satellite_window_spec()
97+ auto spec = WindowSpec{mir_create_surface_spec(connection)}
98+#endif
99+ .set_buffer_usage(mir_buffer_usage_hardware) // Required protobuf field for create_window()
100+ .set_pixel_format(mir_pixel_format_invalid) // Required protobuf field for create_window()
101+ .set_size(width, height)
102+ .set_type(mir_window_type_satellite)
103+ .set_parent(parent);
104+ return spec;
105+ }
106+
107 static auto for_changes(MirConnection* connection) -> WindowSpec
108 {
109 #if MIR_CLIENT_VERSION <= MIR_VERSION_NUMBER(3, 4, 0)
110@@ -144,6 +170,16 @@
111 return *this;
112 }
113
114+ auto set_pixel_format(MirPixelFormat format) -> WindowSpec&
115+ {
116+#if MIR_CLIENT_VERSION <= MIR_VERSION_NUMBER(3, 4, 0)
117+ mir_surface_spec_set_pixel_format(*this, format);
118+#else
119+ mir_window_spec_set_pixel_format(*this, format);
120+#endif
121+ return *this;
122+ }
123+
124 auto set_type(MirWindowType type) -> WindowSpec&
125 {
126 #if MIR_CLIENT_VERSION <= MIR_VERSION_NUMBER(3, 4, 0)
127
128=== modified file 'include/miral/window_manager_tools.h'
129--- include/miral/window_manager_tools.h 2016-11-03 10:15:24 +0000
130+++ include/miral/window_manager_tools.h 2017-02-02 18:03:45 +0000
131@@ -132,6 +132,9 @@
132 /// move the active window
133 void drag_active_window(mir::geometry::Displacement movement);
134
135+ /// move the window
136+ void drag_window(Window const& window, mir::geometry::Displacement movement);
137+
138 /// make the next application active
139 void focus_next_application();
140
141
142=== modified file 'miral/basic_window_manager.cpp'
143--- miral/basic_window_manager.cpp 2017-01-26 15:51:25 +0000
144+++ miral/basic_window_manager.cpp 2017-02-02 18:03:45 +0000
145@@ -999,6 +999,11 @@
146 {
147 auto const window = active_window();
148
149+ drag_window(window, movement);
150+}
151+
152+void miral::BasicWindowManager::drag_window(miral::Window const& window, Displacement& movement)
153+{
154 if (!window)
155 return;
156
157
158=== modified file 'miral/basic_window_manager.h'
159--- miral/basic_window_manager.h 2017-01-13 18:17:01 +0000
160+++ miral/basic_window_manager.h 2017-02-02 18:03:45 +0000
161@@ -120,6 +120,8 @@
162
163 void drag_active_window(mir::geometry::Displacement movement) override;
164
165+ void drag_window(Window const& window, Displacement& movement) override;
166+
167 void focus_next_application() override;
168
169 void focus_next_within_application() override;
170
171=== modified file 'miral/symbols.map'
172--- miral/symbols.map 2017-01-26 11:30:51 +0000
173+++ miral/symbols.map 2017-02-02 18:03:45 +0000
174@@ -349,3 +349,10 @@
175 non-virtual?thunk?to?miral::CanonicalWindowManagerPolicy::place_new_window*;
176 };
177 } MIRAL_1.0;
178+
179+MIRAL_1.2 {
180+global:
181+ extern "C++" {
182+ miral::WindowManagerTools::drag_window*;
183+ };
184+} MIRAL_1.1;
185
186=== modified file 'miral/window_info.cpp'
187--- miral/window_info.cpp 2017-01-26 12:21:51 +0000
188+++ miral/window_info.cpp 2017-02-02 18:03:45 +0000
189@@ -129,12 +129,12 @@
190 case mir_window_type_normal: /**< AKA "regular" */
191 case mir_window_type_utility: /**< AKA "floating" */
192 case mir_window_type_dialog:
193+ case mir_window_type_freestyle:
194+ case mir_window_type_menu:
195+ return true;
196+
197 case mir_window_type_satellite: /**< AKA "toolbox"/"toolbar" */
198- case mir_window_type_freestyle:
199- case mir_window_type_menu:
200 case mir_window_type_inputmethod: /**< AKA "OSK" or handwriting etc. */
201- return true;
202-
203 case mir_window_type_gloss:
204 case mir_window_type_tip: /**< AKA "tooltip" */
205 default:
206
207=== modified file 'miral/window_management_trace.cpp'
208--- miral/window_management_trace.cpp 2017-01-26 11:30:51 +0000
209+++ miral/window_management_trace.cpp 2017-02-02 18:03:45 +0000
210@@ -476,6 +476,17 @@
211 }
212 MIRAL_TRACE_EXCEPTION
213
214+void miral::WindowManagementTrace::drag_window(Window const& window, mir::geometry::Displacement& movement)
215+try {
216+ log_input();
217+ std::stringstream out;
218+ out << movement;
219+ mir::log_info("%s window=%s -> %s", __func__, dump_of(window).c_str(), out.str().c_str());
220+ trace_count++;
221+ wrapped.drag_window(window, movement);
222+}
223+MIRAL_TRACE_EXCEPTION
224+
225 void miral::WindowManagementTrace::focus_next_application()
226 try {
227 log_input();
228
229=== modified file 'miral/window_management_trace.h'
230--- miral/window_management_trace.h 2017-01-26 11:30:51 +0000
231+++ miral/window_management_trace.h 2017-02-02 18:03:45 +0000
232@@ -61,6 +61,8 @@
233
234 virtual void drag_active_window(mir::geometry::Displacement movement) override;
235
236+ void drag_window(Window const& window, mir::geometry::Displacement& movement) override;
237+
238 virtual void focus_next_application() override;
239
240 virtual void focus_next_within_application() override;
241
242=== modified file 'miral/window_manager_tools.cpp'
243--- miral/window_manager_tools.cpp 2016-11-03 10:15:24 +0000
244+++ miral/window_manager_tools.cpp 2017-02-02 18:03:45 +0000
245@@ -62,6 +62,9 @@
246 void miral::WindowManagerTools::drag_active_window(mir::geometry::Displacement movement)
247 { tools->drag_active_window(movement); }
248
249+void miral::WindowManagerTools::drag_window(Window const& window, mir::geometry::Displacement movement)
250+{ tools->drag_window(window, movement); }
251+
252 void miral::WindowManagerTools::focus_next_application()
253 { tools->focus_next_application(); }
254
255
256=== modified file 'miral/window_manager_tools_implementation.h'
257--- miral/window_manager_tools_implementation.h 2016-11-03 10:15:24 +0000
258+++ miral/window_manager_tools_implementation.h 2017-02-02 18:03:45 +0000
259@@ -58,6 +58,7 @@
260 virtual auto active_window() const -> Window = 0;
261 virtual auto select_active_window(Window const& hint) -> Window = 0;
262 virtual void drag_active_window(mir::geometry::Displacement movement) = 0;
263+ virtual void drag_window(Window const& window, mir::geometry::Displacement& movement) = 0;
264 virtual void focus_next_application() = 0;
265 virtual void focus_next_within_application() = 0;
266 virtual auto window_at(mir::geometry::Point cursor) const -> Window = 0;
267
268=== modified file 'scripts/process_doxygen_xml.py'
269--- scripts/process_doxygen_xml.py 2017-01-26 15:51:25 +0000
270+++ scripts/process_doxygen_xml.py 2017-02-02 18:03:45 +0000
271@@ -427,10 +427,18 @@
272 # miral::WindowManagementPolicy::advise_state_change*;
273 _ZN5miral22WindowManagementPolicy19advise_state_changeERKNS_10WindowInfoE14MirWindowState;
274
275+ extern "C++" {
276+ miral::CanonicalWindowManagerPolicy::place_new_window*;
277+ non-virtual?thunk?to?miral::CanonicalWindowManagerPolicy::place_new_window*;
278+ };
279+} MIRAL_1.0;
280+
281+MIRAL_1.2 {
282+global:
283 extern "C++" {'''
284
285 END_NEW_STANZA = ''' };
286-} MIRAL_1.0;'''
287+} MIRAL_1.1;'''
288
289 def _print_report():
290 print OLD_STANZAS
291
292=== modified file 'test/active_window.cpp'
293--- test/active_window.cpp 2017-02-02 18:03:45 +0000
294+++ test/active_window.cpp 2017-02-02 18:03:45 +0000
295@@ -350,3 +350,41 @@
296 EXPECT_TRUE(sync2.signal_raised());
297 assert_active_window_is(dialog_name);
298 }
299+
300+TEST_F(ActiveWindow, input_methods_are_not_focussed)
301+{
302+ char const* const test_name = __PRETTY_FUNCTION__;
303+ auto const connection = connect_client(test_name);
304+
305+ auto const parent = create_surface(connection, test_name, sync1);
306+ auto const input_method = WindowSpec::for_input_method(connection, 50, 50, parent).create_window();
307+
308+ assert_active_window_is(test_name);
309+
310+ invoke_tools([&](WindowManagerTools& tools)
311+ {
312+ auto const& info = tools.info_for(tools.active_window());
313+ tools.select_active_window(info.children().at(0));
314+ });
315+
316+ assert_active_window_is(test_name);
317+}
318+
319+TEST_F(ActiveWindow, satellites_are_not_focussed)
320+{
321+ char const* const test_name = __PRETTY_FUNCTION__;
322+ auto const connection = connect_client(test_name);
323+
324+ auto const parent = create_surface(connection, test_name, sync1);
325+ auto const satellite = WindowSpec::for_satellite(connection, 50, 50, parent).create_window();
326+
327+ assert_active_window_is(test_name);
328+
329+ invoke_tools([&](WindowManagerTools& tools)
330+ {
331+ auto const& info = tools.info_for(tools.active_window());
332+ tools.select_active_window(info.children().at(0));
333+ });
334+
335+ assert_active_window_is(test_name);
336+}
337
338=== modified file 'test/drag_active_window.cpp'
339--- test/drag_active_window.cpp 2017-01-13 18:17:01 +0000
340+++ test/drag_active_window.cpp 2017-02-02 18:03:45 +0000
341@@ -110,32 +110,46 @@
342 // Freestyle surfaces may or may not be, as specified by the app.
343 // Mir and Unity: Surfaces, input, and displays (v0.3)
344 INSTANTIATE_TEST_CASE_P(DragActiveWindow, ForMoveableTypes, ::testing::Values(
345- mir_surface_type_normal,
346- mir_surface_type_utility,
347- mir_surface_type_dialog,
348-// mir_surface_type_overlay,
349-// mir_surface_type_gloss,
350- mir_surface_type_freestyle,
351-// mir_surface_type_popover,
352-// mir_surface_type_menu,
353-// mir_surface_type_inputmethod,
354- mir_surface_type_satellite
355-// mir_surface_type_tip,
356-// mir_surface_types
357+ mir_window_type_normal,
358+ mir_window_type_utility,
359+ mir_window_type_dialog,
360+// mir_window_type_gloss,
361+ mir_window_type_freestyle
362+// mir_window_type_menu,
363+// mir_window_type_inputmethod,
364+// mir_window_type_satellite,
365+// mir_window_type_tip,
366+// mir_window_types
367 ));
368
369
370 INSTANTIATE_TEST_CASE_P(DragActiveWindow, ForUnmoveableTypes, ::testing::Values(
371-// mir_surface_type_normal,
372-// mir_surface_type_utility,
373-// mir_surface_type_dialog,
374- mir_surface_type_overlay,
375- mir_surface_type_gloss,
376-// mir_surface_type_freestyle,
377- mir_surface_type_popover,
378- mir_surface_type_menu,
379- mir_surface_type_inputmethod,
380-// mir_surface_type_satellite,
381- mir_surface_type_tip
382-// mir_surface_types
383+// mir_window_type_normal,
384+// mir_window_type_utility,
385+// mir_window_type_dialog,
386+ mir_window_type_gloss,
387+// mir_window_type_freestyle,
388+ mir_window_type_menu,
389+ mir_window_type_inputmethod,
390+// mir_window_type_satellite,
391+ mir_window_type_tip
392+// mir_window_types
393 ));
394+
395+using DragWindow = DragActiveWindow;
396+
397+TEST_F(DragWindow, can_drag_satellite)
398+{
399+ create_window_of_type(mir_window_type_satellite);
400+
401+ Displacement const movement{10, 10};
402+ auto const initial_position = window.top_left();
403+ auto const expected_position = initial_position + movement;
404+
405+ EXPECT_CALL(*window_manager_policy, advise_move_to(_, expected_position));
406+
407+ window_manager_tools.drag_window(window, movement);
408+
409+ EXPECT_THAT(window.top_left(), Eq(expected_position))
410+ << "Type: " << GetParam();
411+}
412\ No newline at end of file

Subscribers

People subscribed via source and target branches