Re: [PATCH gtk-3-16] GtkComboBox wrong menu_ypos

[Leslie Zhai <xiang zhai i-soft com cn>]

Hi Emmanuele,

Thanks for your reply ;-)

I open a bug https://bugzilla.gnome.org/show_bug.cgi?id=750372
Please pay some attention to my patch, thanks a lot!

On 2015年06月03日 17:13, Emmanuele Bassi wrote:
> Hi;
>
> Thanks for looking into this.
>
> GTK+ uses Bugzilla to track features, bugs, and more importantly patches.
>
> Please, open a bug on bugzilla.gnome.org:
>
>    https://bugzilla.gnome.org/enter_bug.cgi?product=gtk%2B&component=Widget:%20GtkComboBox
>
> And attach your patch there.
>
> Thanks for your contribution!
>
> Ciao,
>   Emmanuele.
>
>
> On 3 June 2015 at 05:16, Leslie Zhai <xiang zhai i-soft com cn> wrote:
> > Implemented when active_item is not zero ;-)
> >
> > When X11`s DisplayHeight minus GdkEventButton`s y_root is smaller than popup
> > menu allocation`s height,
> > menu_ypos -= allocation.height;
> >
> > But when active_item is bigger than zero, for example, pressed combo popup
> > menu`s item 8,
> > menu_ypos += active_item * child_allocation.height;
> >
> >
> > diff --git a/gtk/gtkcombobox.c b/gtk/gtkcombobox.c
> > index ccda6ad..5f46920 100644
> > --- a/gtk/gtkcombobox.c
> > +++ b/gtk/gtkcombobox.c
> > @@ -159,6 +159,9 @@ struct _GtkComboBoxPrivate
> >     GdkDevice *grab_keyboard;
> >
> >     gchar *tearoff_title;
> > +
> > +  guint y_root;
> > +  gint active_item;
> >   };
> >
> >   /* While debugging this evil code, I have learned that
> > @@ -1125,6 +1128,9 @@ gtk_combo_box_init (GtkComboBox *combo_box)
> >     priv->text_column = -1;
> >     priv->text_renderer = NULL;
> >     priv->id_column = -1;
> > +
> > +  priv->y_root = 0;
> > +  priv->active_item = -1;
> >   }
> >
> >   static void
> > @@ -1898,6 +1904,18 @@ gtk_combo_box_menu_position_below (GtkMenu *menu,
> >      *push_in = FALSE;
> >   }
> >
> > +static GdkRectangle m_get_monitor_geo()
> > +{
> > +  GdkRectangle r;
> > +  GdkDisplay *display = gdk_display_get_default();
> > +  GdkScreen *screen = gdk_display_get_default_screen(display);
> > +  gint mon_id = gdk_screen_get_primary_monitor(screen);
> > +
> > +  gdk_screen_get_monitor_geometry(screen, mon_id, &r);
> > +
> > +  return r;
> > +}
> > +
> >   static void
> >   gtk_combo_box_menu_position_over (GtkMenu  *menu,
> >                                     gint     *x,
> > @@ -1956,6 +1974,14 @@ gtk_combo_box_menu_position_over (GtkMenu *menu,
> >         children = children->next;
> >       }
> >
> > +  gtk_widget_get_allocation(GTK_WIDGET(menu), &allocation);
> > +  GdkRectangle display_rect = m_get_monitor_geo();
> > +  if (display_rect.height - combo_box->priv->y_root < allocation.height) {
> > +    menu_ypos -= allocation.height;
> > +    if (combo_box->priv->active_item != -1)
> > +      menu_ypos += combo_box->priv->active_item * child_allocation.height;
> > +  }
> > +
> >     if (gtk_widget_get_direction (widget) == GTK_TEXT_DIR_RTL)
> >       menu_xpos = menu_xpos + allocation.width - menu_width;
> >
> > @@ -2244,6 +2270,7 @@ gtk_combo_box_menu_popup (GtkComboBox *combo_box,
> >       }
> >
> >     /* FIXME handle nested menus better */
> > +  priv->active_item = active_item;
> >     gtk_menu_set_active (GTK_MENU (priv->popup_widget), active_item);
> >
> >     if (priv->wrap_width == 0)
> > @@ -3237,6 +3264,8 @@ gtk_combo_box_menu_button_press (GtkWidget
> > *widget,
> >             !gtk_widget_has_focus (priv->button))
> >           gtk_widget_grab_focus (priv->button);
> >
> > +      priv->y_root = event->y_root;
> > +
> >         gtk_combo_box_menu_popup (combo_box, event->button, event->time);
> >
> >         return TRUE;
> >
> >
> > On 2015年06月03日 09:52, Leslie Zhai wrote:
> > > Hi Gtk developers,
> > >
> > > I pay more attention to the gtk_combo_box_menu_position, the user supplied
> > > function used to position the menu, when pressed the combobox button
> > > https://git.gnome.org/browse/gtk+/tree/gtk/gtkcombobox.c?h=gtk-3-16#n2268
> > >
> > > I argue that it ignore the relationship between DisplayHeight (use X11
> > > style), GdkEventButton`s y_root and popup menu`s allocation height
> > > https://twitter.com/xiangzhai/status/605902462189314048
> > >
> > > If DisplayHeight minus y_root is smaller than menu_alloc_height, the
> > > menu_ypos should be minus menu_alloc_height, then my workaround patch shown
> > > as below, not a monkey patch any more ;)
> > > https://mail.gnome.org/archives/gtk-devel-list/2015-June/msg00000.html
> > >
> > >
> > > diff --git a/gtk/gtkcombobox.c b/gtk/gtkcombobox.c
> > > index ccda6ad..ce60473 100644
> > > --- a/gtk/gtkcombobox.c
> > > +++ b/gtk/gtkcombobox.c
> > > @@ -159,6 +159,7 @@ struct _GtkComboBoxPrivate
> > >     GdkDevice *grab_keyboard;
> > >
> > >     gchar *tearoff_title;
> > > +  guint y_root;
> > >   };
> > >
> > >   /* While debugging this evil code, I have learned that
> > > @@ -1125,6 +1126,8 @@ gtk_combo_box_init (GtkComboBox *combo_box)
> > >     priv->text_column = -1;
> > >     priv->text_renderer = NULL;
> > >     priv->id_column = -1;
> > > +
> > > +  priv->y_root = 0;
> > >   }
> > >
> > >   static void
> > > @@ -1898,6 +1901,18 @@ gtk_combo_box_menu_position_below (GtkMenu *menu,
> > >      *push_in = FALSE;
> > >   }
> > >
> > > +static GdkRectangle m_get_monitor_geo()
> > > +{
> > > +  GdkRectangle r;
> > > +  GdkDisplay *display = gdk_display_get_default();
> > > +  GdkScreen *screen = gdk_display_get_default_screen(display);
> > > +  gint mon_id = gdk_screen_get_primary_monitor(screen);
> > > +
> > > +  gdk_screen_get_monitor_geometry(screen, mon_id, &r);
> > > +
> > > +  return r;
> > > +}
> > > +
> > >   static void
> > >   gtk_combo_box_menu_position_over (GtkMenu  *menu,
> > >                                     gint     *x,
> > > @@ -1938,6 +1953,13 @@ gtk_combo_box_menu_position_over (GtkMenu *menu,
> > >         menu_ypos -= child_allocation.height / 2;
> > >       }
> > >
> > > +  gtk_widget_get_allocation(GTK_WIDGET(menu), &allocation);
> > > +  GdkRectangle display_rect = m_get_monitor_geo();
> > > +  if (display_rect.height - combo_box->priv->y_root < allocation.height)
> > > {
> > > +    menu_ypos -= allocation.height;
> > > +  }
> > > +
> > >     children = GTK_MENU_SHELL
> > > (combo_box->priv->popup_widget)->priv->children;
> > >     while (children)
> > >       {
> > > @@ -3237,6 +3261,7 @@ gtk_combo_box_menu_button_press (GtkWidget
> > > *widget,
> > >             !gtk_widget_has_focus (priv->button))
> > >           gtk_widget_grab_focus (priv->button);
> > >
> > > +      priv->y_root = event->y_root;
> > >         gtk_combo_box_menu_popup (combo_box, event->button, event->time);
> > >
> > >         return TRUE;
> > >
> > >
> > > What about ***unimplented*** part? menu_ypos minus popup menu`s allocation
> > > height is not enough when popup menu ***active*** item is, for example, last
> > > one not the first, then menu_ypos should be plus the height of active item
> > > to the first one, sorry for my poor English ;P
> > --
> > Regards,
> > Leslie Zhai
> >
> >
> >
> > _______________________________________________
> > gtk-devel-list mailing list
> > gtk-devel-list gnome org
> > https://mail.gnome.org/mailman/listinfo/gtk-devel-list

--
Regards,
Leslie Zhai