GNOME Bugzilla – Bug 678405
pre-overview miscellaneous code cleanups
Last modified: 2012-06-20 05:48:50 UTC
These patches reorganize a bit the code and add a few things we will need for the overview. They are fairly orthogonal to the overview changes so they could go in in any case.
Created attachment 216742 [details] [review] ephy-window: add _ephy_window_set_navigation_flags() We'll share this code later.
Created attachment 216743 [details] [review] ephy-window: split ephy_window_set_active_tab() into smaller methods Which we will also reuse later on.
Created attachment 216744 [details] [review] ephy-location-entry: make it possible to hide the favicon The favicon is not needed when the overview is visible.
Created attachment 216745 [details] [review] ephy-location-controller: add a boolean "show-icon" property When the overview is visible, we don't need to show the favicon
Created attachment 216746 [details] [review] ephy-window: add a method to toggle visibility of default actions Where default actions are the ones that should be disabled when the overview or a blank page are visible.
Review of attachment 216742 [details] [review]: Looks good.
Review of attachment 216743 [details] [review]: OK.
Review of attachment 216744 [details] [review]: OK.
Review of attachment 216745 [details] [review]: OK. ::: src/ephy-location-controller.c @@ +592,3 @@ + "If we should show the favicon", + TRUE, + G_PARAM_CONSTRUCT | G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); Nitpick: s/If/Whether/ in both.
Review of attachment 216745 [details] [review]: OK, but this does not actually bind the overview visibility to the icon visibility right? The commit message is a bit misguiding.
Review of attachment 216746 [details] [review]: Hrm, if the flag used here is always SENS_FLAG_IS_BLANK shouldn't it be just hardcoded? Does this become configurable at some point? I find a bit confusing the "flags" name when you always pass the same thing.
(In reply to comment #10) > Review of attachment 216745 [details] [review]: > > OK, but this does not actually bind the overview visibility to the icon > visibility right? The commit message is a bit misguiding. Maybe I should remove the mention to the overview, since it's not-existent yet. Once we add it we bind both. (In reply to comment #11) > Review of attachment 216746 [details] [review]: > > Hrm, if the flag used here is always SENS_FLAG_IS_BLANK shouldn't it be just > hardcoded? Does this become configurable at some point? I find a bit confusing > the "flags" name when you always pass the same thing. We later use this method with SENS_FLAG_IS_OVERVIEW or something like that, that's the point of factoring the code out now.
Review of attachment 216746 [details] [review]: OK then, I just needed that context to figure this out.