fix crash when close owner window

window position issue makes popup menu at wrong position

include/nana/gui/widgets/menubar.hpp

@@ -1,7 +1,7 @@
 /*
  *	A Menubar implementation
  *	Nana C++ Library(http://www.nanapro.org)
- *	Copyright(C) 2009-2017 Jinhao(cnjinhao@hotmail.com)
+ *	Copyright(C) 2009-2018 Jinhao(cnjinhao@hotmail.com)
  *
  *	Distributed under the Boost Software License, Version 1.0.
  *	(See accompanying file LICENSE_1_0.txt or copy at
@@ -34,26 +34,6 @@ namespace nana
 				color_proxy border_highlight{ colors::highlight };
 			};
 
-			class item_renderer
-			{
-			public:
-				enum class state
-				{
-					normal, highlighted, selected
-				};
-
-				using graph_reference = paint::graphics&;
-				using scheme = ::nana::drawerbase::menubar::scheme;
-
-				item_renderer(window, graph_reference);
-				virtual void background(const point&, const ::nana::size&, state);
-				virtual void caption(const point&, const native_string_type&);
-				scheme *scheme_ptr() const { return scheme_ptr_; };
-			private:
-				graph_reference graph_;
-				scheme *scheme_ptr_;
-			};
-
 			class trigger
 				: public drawer_trigger
 			{

source/detail/platform_spec_posix.cpp

@@ -658,21 +658,31 @@ namespace detail
 		platform_scope_guard lock;
 		if(umake_owner(wd))
 		{
+			auto & wd_manager = detail::bedrock::instance().wd_manager();
+
+			std::vector<native_window_type> owned_children;
+
 			auto i = wincontext_.find(wd);
 			if(i != wincontext_.end())
 			{
 				if(i->second.owned)
 				{
-					set_error_handler();
+					for(auto child : *i->second.owned)
-					auto & wd_manager = detail::bedrock::instance().wd_manager();
+						owned_children.push_back(child);
-					for(auto u = i->second.owned->rbegin(); u != i->second.owned->rend(); ++u)
-						wd_manager.close(wd_manager.root(*u));
-
-					rev_error_handler();
-
-					delete i->second.owned;
 				}
+			}
 
+			//Closing a child will erase the wd from the table wincontext_, so the 
+			//iterator i can't be reused after children closed.
+			set_error_handler();
+			for(auto u = owned_children.rbegin(); u != owned_children.rend(); ++u)
+				wd_manager.close(wd_manager.root(*u));
+			rev_error_handler();
+
+			i = wincontext_.find(wd);
+			if(i != wincontext_.end())
+			{
+				delete i->second.owned;
 				wincontext_.erase(i);
 			}
 		}

source/gui/detail/native_window_interface.cpp

@@ -371,6 +371,8 @@ namespace nana{
 			}
 
 			Window parent = (owner ? reinterpret_cast<Window>(owner) : restrict::spec.root_window());
+
+			//The position passed to XCreateWindow is a screen coordinate.
 			nana::point pos(r.x, r.y);
 			if((false == nested) && owner)
 			{
@@ -396,7 +398,9 @@ namespace nana{
 				{
 					auto origin_owner = (owner ? owner : reinterpret_cast<native_window_type>(restrict::spec.root_window()));
 					restrict::spec.make_owner(origin_owner, reinterpret_cast<native_window_type>(handle));
-					exposed_positions[handle] = pos;
+
+					//The exposed_position is a relative position to its owner/parent.
+					exposed_positions[handle] = r.position();
 				}
 
 				XChangeWindowAttributes(disp, handle, attr_mask, &win_attr);
@@ -945,13 +949,14 @@ namespace nana{
 				auto fm_extents = window_frame_extents(wd);
 				origin.x = -fm_extents.left;
 				origin.y = -fm_extents.top;
-
+#if 0	//deprecated
 				if(reinterpret_cast<Window>(coord_wd) != restrict::spec.root_window())
 				{
 					fm_extents = window_frame_extents(coord_wd);
 					origin.x += fm_extents.left;
 					origin.y += fm_extents.top;
 				}
+#endif
 			}
 			else
 				coord_wd = get_window(wd, window_relationship::parent);
@@ -1010,8 +1015,14 @@ namespace nana{
 			if(owner && (owner != reinterpret_cast<native_window_type>(restrict::spec.root_window())))
 			{
 				auto origin = window_position(owner);
+#if 0
 				x += origin.x;
 				y += origin.y;
+#else
+				auto owner_extents = window_frame_extents(owner);
+				x += origin.x + owner_extents.left;
+				y += origin.y + owner_extents.top;
+#endif
 			}
 
 			::XMoveWindow(disp, reinterpret_cast<Window>(wd), x, y);
@@ -1099,8 +1110,14 @@ namespace nana{
 			if(owner && (owner != reinterpret_cast<native_window_type>(restrict::spec.root_window())))
 			{
 				auto origin = window_position(owner);
+#if 0
 				x += origin.x;
 				y += origin.y;
+#else
+				auto owner_extents = window_frame_extents(owner);
+				x += origin.x + owner_extents.left;
+				y += origin.y + owner_extents.top;
+#endif
 			}
 
 			::XMoveResizeWindow(disp, reinterpret_cast<Window>(wd), x, y, r.width, r.height);
@@ -1580,14 +1597,21 @@ namespace nana{
 				pos.y = point.y;
 				return true;
 			}
-			return false;
 #elif defined(NANA_X11)
 			nana::detail::platform_scope_guard psg;
 			int x = pos.x, y = pos.y;
 			Window child;
-			return (True == ::XTranslateCoordinates(restrict::spec.open_display(),
+			if(True == ::XTranslateCoordinates(restrict::spec.open_display(),
-													reinterpret_cast<Window>(wd), restrict::spec.root_window(), x, y, &pos.x, &pos.y, &child));
+													reinterpret_cast<Window>(wd), restrict::spec.root_window(), x, y, &pos.x, &pos.y, &child))
+			{
+				//deprecated
+				//auto fm_extents = window_frame_extents(wd);
+				//pos.x += fm_extents.left;
+				//pos.y += fm_extents.top;
+				return true;
+			}
 #endif
+			return false;
 		}
 
 		bool native_interface::calc_window_point(native_window_type wd, nana::point& pos)
@@ -1600,14 +1624,21 @@ namespace nana{
 				pos.y = point.y;
 				return true;
 			}
-			return false;
 #elif defined(NANA_X11)
 			nana::detail::platform_scope_guard psg;
 			int x = pos.x, y = pos.y;
 			Window child;
-			return (True == ::XTranslateCoordinates(restrict::spec.open_display(),
+			if(True == ::XTranslateCoordinates(restrict::spec.open_display(), restrict::spec.root_window(), reinterpret_cast<Window>(wd), x, y, &pos.x, &pos.y, &child))
-													restrict::spec.root_window(), reinterpret_cast<Window>(wd), x, y, &pos.x, &pos.y, &child));
+			{
+				//deprecated
+				//Now the origin of pos is the left-top corner of the window(including titlebar and border)
+				//auto fm_extents = window_frame_extents(wd);
+				//pos.x += fm_extents.left;
+				//pos.y += fm_extents.top;
+				return true;
+			}
 #endif
+			return false;
 		}
 
 		native_window_type native_interface::find_window(int x, int y)

source/gui/widgets/menubar.cpp

@@ -1,7 +1,7 @@
 /*
 *	A Menubar implementation
 *	Nana C++ Library(http://www.nanapro.org)
-*	Copyright(C) 2009-2017 Jinhao(cnjinhao@hotmail.com)
+*	Copyright(C) 2009-2018 Jinhao(cnjinhao@hotmail.com)
 *
 *	Distributed under the Boost Software License, Version 1.0.
 *	(See accompanying file LICENSE_1_0.txt or copy at
@@ -215,12 +215,32 @@ namespace nana
 			};
 
 			//class item_renderer
-				item_renderer::item_renderer(window wd, graph_reference graph)
+			class item_renderer
-					:graph_(graph), scheme_ptr_(static_cast<scheme*>(API::dev::get_scheme(wd)))
+			{
-				{}
+			public:
-
+				enum class state
-				void item_renderer::background(const nana::point& pos, const nana::size& size, state item_state)
 				{
+					normal, highlighted, selected
+				};
+
+				using graph_reference = paint::graphics&;
+				using scheme = ::nana::drawerbase::menubar::scheme;
+
+				item_renderer(window, graph_reference);
+				virtual void background(const point&, const ::nana::size&, state);
+				virtual void caption(const point&, const native_string_type&);
+				scheme *scheme_ptr() const { return scheme_ptr_; };
+			private:
+				graph_reference graph_;
+				scheme *scheme_ptr_;
+			};
+
+			item_renderer::item_renderer(window wd, graph_reference graph)
+				:graph_(graph), scheme_ptr_(static_cast<scheme*>(API::dev::get_scheme(wd)))
+			{}
+
+			void item_renderer::background(const nana::point& pos, const nana::size& size, state item_state)
+			{
 					auto bground = scheme_ptr_->text_fgcolor;
 					::nana::color border, body;
 
@@ -245,13 +265,12 @@ namespace nana
 
 					paint::draw{ graph_ }.corner(r, 1);
 					graph_.rectangle(r.pare_off(1), true, body);
-				}
+			}
 
-				void item_renderer::caption(const point& pos, const native_string_type& text)
+			void item_renderer::caption(const point& pos, const native_string_type& text)
-				{
+			{
-					graph_.string(pos, text, scheme_ptr_->text_fgcolor);
+				graph_.string(pos, text, scheme_ptr_->text_fgcolor);
-
+			}
-				}
 			//end class item_renderer
 
 			//class trigger