Merge lp:~azzar1/unity/fix-868452-825037 into lp:unity

[Andrea Azzarone]

Display a list of the lenses in the bfb quicklist. It tries to fix bug #825037 too.

[Marco Trevisan (Treviño) (3v1n0)]

It works as advertised, I guess this should be tested when autopilot is ready.

However, works well (fixes both the linked bugs) and the code looks good to me.

Just one thing, I guess that should be fixed at upper levels:
+ // "Files & folders", you are the culprit!
+ std::string name = lens->name();
+ boost::replace_all(name, "&", "&");

Plus, if you open the dash, and you right-click on the BFB, all the quicklist items works, except the "Dash Home" item.

review: Needs Fixing

[Andrea Azzarone (azzar1)]

> It works as advertised, I guess this should be tested when autopilot is ready.
>
> However, works well (fixes both the linked bugs) and the code looks good to
> me.
>
> Just one thing, I guess that should be fixed at upper levels:
> + // "Files & folders", you are the culprit!
> + std::string name = lens->name();
> + boost::replace_all(name, "&", "&");

In the quicklist drawing functions?

>
> Plus, if you open the dash, and you right-click on the BFB, all the quicklist
> items works, except the "Dash Home" item.

I'll talk to Neil about this ;)

[Marco Trevisan (Treviño) (3v1n0)]

> > Just one thing, I guess that should be fixed at upper levels:
> > + // "Files & folders", you are the culprit!
> > + std::string name = lens->name();
> > + boost::replace_all(name, "&", "&");
>
> In the quicklist drawing functions?

Sorry, I meant "lower" level. So, I guess on the Unity-core lens wrapper or even at the libunity level.
I guess that libunity replaces the & with & to send these values via dbus, so we just need to check that the lens->name() property returns the right value for that.

[Andrea Azzarone (azzar1)]

> > > Just one thing, I guess that should be fixed at upper levels:
> > > + // "Files & folders", you are the culprit!
> > > + std::string name = lens->name();
> > > + boost::replace_all(name, "&", "&");
> >
> > In the quicklist drawing functions?
>
> Sorry, I meant "lower" level. So, I guess on the Unity-core lens wrapper or
> even at the libunity level.
> I guess that libunity replaces the & with & to send these values via dbus,
> so we just need to check that the lens->name() property returns the right
> value for that.

I don't understand. The problem is that lens->name() returns "Files & folders" and not "Files & Folder". I think that we're using pango in the quicklist drawing functions and pango doesn't like the & char :)

[Marco Trevisan (Treviño) (3v1n0)]

Ah, ok... Sorry I misunderstood that then.
So, the issue is on the quicklist drawing function, and the issue is happening also when a quicklist item has a "<" inside.

This should be definitely fixed at that level, otherwise also the application quicklist will suffer for it.
I'll open another bug for that and I'll work on it, so please, remove the workaround from your code :)

[Andrea Azzarone (azzar1)]

Ok... Use something like this:

       boost::replace_all(value, "&", "&amp;");
       boost::replace_all(value, "<", "&lt;");
       boost::replace_all(value, ">", "&gt;");

[Marco Trevisan (Treviño) (3v1n0)]

Andrea, to fix both the & and < issue, you can just use g_markup_escape_text(str, -1), then when I'll fix properly the bug #899677 (easy to fix, but as discussed, I'd like to define something with design before), I'll see if this should centralized.

[Andrea Azzarone (azzar1)]

Done.

[Marco Trevisan (Treviño) (3v1n0)]

Code looks good now, and works as expected.

However I guess you need at least a manual test for it.

review: Approve

[Mirco Müller (macslow)]

Ok, looking good to me.

review: Approve

[Unity Merger (unity-merger)]

Attempt to merge into lp:unity failed due to conflicts:

text conflict in plugins/unityshell/src/BFBLauncherIcon.cpp

[Andrea Azzarone (azzar1)]

All conflicts resolved.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/35/console reported an error when processing this lp:~andyrock/unity/fix-868452-825037 branch.
Not merging it.

=== modified file 'plugins/unityshell/src/BFBLauncherIcon.cpp'
--- plugins/unityshell/src/BFBLauncherIcon.cpp	2011-11-22 17:47:59 +0000
+++ plugins/unityshell/src/BFBLauncherIcon.cpp	2011-12-06 16:36:28 +0000
@@ -15,6 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Jason Smith <[e-mail]>
+ *              Andrea Azzarone <[e-mail]>
  */
 
 #include "BFBLauncherIcon.h"
@@ -28,6 +29,8 @@
 {
 namespace launcher
 {
+  
+UBusManager BFBLauncherIcon::ubus_manager_;
 
 BFBLauncherIcon::BFBLauncherIcon(Launcher* IconManager)
  : SimpleLauncherIcon(IconManager)
@@ -37,29 +40,84 @@
   SetQuirk(QUIRK_VISIBLE, true);
   SetQuirk(QUIRK_RUNNING, false);
   SetIconType(TYPE_HOME);
-
-  _background_color = nux::color::White;
-
-  mouse_enter.connect([&] () { _ubus_manager.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });
+  
+  background_color_ = nux::color::White;
+  
+  mouse_enter.connect([&]() { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });
 }
 
 nux::Color BFBLauncherIcon::BackgroundColor()
 {
-  return _background_color;
+  return background_color_;
 }
 
 nux::Color BFBLauncherIcon::GlowColor()
 {
-  return _background_color;
+  return background_color_;
 }
 
 void BFBLauncherIcon::ActivateLauncherIcon(ActionArg arg)
 {
-  if (arg.button == 1)
-    _ubus_manager.SendMessage (UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", "home.lens", 0, ""));
+  ubus_manager_.SendMessage(UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", "home.lens", 0, ""));
 
   // dont chain down to avoid random dash close events
 }
 
+void BFBLauncherIcon::OnMenuitemActivated(DbusmenuMenuitem* item,
+                                          int time,
+                                          gchar* lens)
+{
+  if (lens != NULL)
+  {
+    ubus_manager_.SendMessage(UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", lens, dash::GOTO_DASH_URI, ""));
+    g_free(lens);
+  }
+}                                     
+
+std::list<DbusmenuMenuitem*> BFBLauncherIcon::GetMenus()
+{  
+  std::list<DbusmenuMenuitem*> result;
+  DbusmenuMenuitem* menu_item;
+  
+  // Home dash
+  menu_item = dbusmenu_menuitem_new();
+  
+  dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Dash Home"));
+  dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
+  dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+  
+  g_signal_connect(menu_item,
+                   DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                   (GCallback)&BFBLauncherIcon::OnMenuitemActivated,
+                   g_strdup("home.lens"));
+  
+  result.push_back(menu_item);
+  
+  // Other lenses..
+  for (auto lens : lenses_.GetLenses())
+  {
+    if (!lens->visible())
+      continue;
+    
+    glib::String name(g_markup_escape_text(lens->name().c_str(), -1));
+    
+    menu_item = dbusmenu_menuitem_new();
+
+    dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, name.Value());
+    dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
+    dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+
+    g_signal_connect(menu_item,
+                     DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                     (GCallback)&BFBLauncherIcon::OnMenuitemActivated,
+                     g_strdup(lens->id().c_str()));
+                     
+    result.push_back(menu_item);
+  }
+  
+  return result;
+}
+
 } // namespace launcher
 } // namespace unity
+

=== modified file 'plugins/unityshell/src/BFBLauncherIcon.h'
--- plugins/unityshell/src/BFBLauncherIcon.h	2011-10-06 04:18:36 +0000
+++ plugins/unityshell/src/BFBLauncherIcon.h	2011-12-06 16:36:28 +0000
@@ -22,6 +22,8 @@
 
 #include "SimpleLauncherIcon.h"
 
+#include <UnityCore/FilesystemLenses.h>
+
 #include "UBusWrapper.h"
 
 namespace unity
@@ -39,9 +41,16 @@
   virtual nux::Color GlowColor();
 
   void ActivateLauncherIcon(ActionArg arg);
+
+protected:
+  std::list<DbusmenuMenuitem*> GetMenus();
+
 private:
-  unity::UBusManager _ubus_manager;
-  nux::Color _background_color;
+  static void OnMenuitemActivated(DbusmenuMenuitem* item, int time, gchar* lens);
+                                     
+  static unity::UBusManager ubus_manager_;
+  nux::Color background_color_;
+  dash::FilesystemLenses lenses_;
 };
 
 }

=== modified file 'plugins/unityshell/src/DashView.cpp'
--- plugins/unityshell/src/DashView.cpp	2011-12-06 00:21:09 +0000
+++ plugins/unityshell/src/DashView.cpp	2011-12-06 16:36:28 +0000
@@ -542,16 +542,16 @@
 {
   glib::String uri;
   glib::String search_string;
+  dash::HandledType handled_type;
 
-  g_variant_get(args, "(sus)", &uri, NULL, &search_string);
+  g_variant_get(args, "(sus)", &uri, &handled_type, &search_string);
 
   std::string id = AnalyseLensURI(uri.Str());
 
   home_view_->search_string = "";
   lens_bar_->Activate(id);
 
-
-  if (id == "home.lens" || !visible_)
+  if ((id == "home.lens" && handled_type != GOTO_DASH_URI ) || !visible_)
     ubus_manager_.SendMessage(UBUS_DASH_EXTERNAL_ACTIVATION);
 }