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

Proposed by Andrea Azzarone
Status: Merged
Approved by: Andrea Azzarone
Approved revision: no longer in the source branch.
Merged at revision: 1764
Proposed branch: lp:~azzar1/unity/fix-868452-825037
Merge into: lp:unity
Diff against target: 168 lines (+80/-13)
3 files modified
plugins/unityshell/src/BFBLauncherIcon.cpp (+66/-8)
plugins/unityshell/src/BFBLauncherIcon.h (+11/-2)
plugins/unityshell/src/DashView.cpp (+3/-3)
To merge this branch: bzr merge lp:~azzar1/unity/fix-868452-825037
Reviewer Review Type Date Requested Status
Mirco Müller (community) Approve
Marco Trevisan (Treviño) Approve
Review via email: mp+83652@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> 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 ;)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> > 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.

Revision history for this message
Andrea Azzarone (azzar1) wrote :

> > > 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 :)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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 :)

Revision history for this message
Andrea Azzarone (azzar1) wrote :

Ok... Use something like this:

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

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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.

Revision history for this message
Andrea Azzarone (azzar1) wrote :

Done.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Code looks good now, and works as expected.

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

review: Approve
Revision history for this message
Mirco Müller (macslow) wrote :

Ok, looking good to me.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

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

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

Revision history for this message
Andrea Azzarone (azzar1) wrote :

All conflicts resolved.

Revision history for this message
Unity Merger (unity-merger) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
15 * along with this program. If not, see <http://www.gnu.org/licenses/>.15 * along with this program. If not, see <http://www.gnu.org/licenses/>.
16 *16 *
17 * Authored by: Jason Smith <jason.smith@canonical.com>17 * Authored by: Jason Smith <jason.smith@canonical.com>
18 * Andrea Azzarone <azzaronea@gmail.com>
18 */19 */
1920
20#include "BFBLauncherIcon.h"21#include "BFBLauncherIcon.h"
@@ -28,6 +29,8 @@
28{29{
29namespace launcher30namespace launcher
30{31{
32
33UBusManager BFBLauncherIcon::ubus_manager_;
3134
32BFBLauncherIcon::BFBLauncherIcon(Launcher* IconManager)35BFBLauncherIcon::BFBLauncherIcon(Launcher* IconManager)
33 : SimpleLauncherIcon(IconManager)36 : SimpleLauncherIcon(IconManager)
@@ -37,29 +40,84 @@
37 SetQuirk(QUIRK_VISIBLE, true);40 SetQuirk(QUIRK_VISIBLE, true);
38 SetQuirk(QUIRK_RUNNING, false);41 SetQuirk(QUIRK_RUNNING, false);
39 SetIconType(TYPE_HOME);42 SetIconType(TYPE_HOME);
4043
41 _background_color = nux::color::White;44 background_color_ = nux::color::White;
4245
43 mouse_enter.connect([&] () { _ubus_manager.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });46 mouse_enter.connect([&]() { ubus_manager_.SendMessage(UBUS_DASH_ABOUT_TO_SHOW, NULL); });
44}47}
4548
46nux::Color BFBLauncherIcon::BackgroundColor()49nux::Color BFBLauncherIcon::BackgroundColor()
47{50{
48 return _background_color;51 return background_color_;
49}52}
5053
51nux::Color BFBLauncherIcon::GlowColor()54nux::Color BFBLauncherIcon::GlowColor()
52{55{
53 return _background_color;56 return background_color_;
54}57}
5558
56void BFBLauncherIcon::ActivateLauncherIcon(ActionArg arg)59void BFBLauncherIcon::ActivateLauncherIcon(ActionArg arg)
57{60{
58 if (arg.button == 1)61 ubus_manager_.SendMessage(UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", "home.lens", 0, ""));
59 _ubus_manager.SendMessage (UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", "home.lens", 0, ""));
6062
61 // dont chain down to avoid random dash close events63 // dont chain down to avoid random dash close events
62}64}
6365
66void BFBLauncherIcon::OnMenuitemActivated(DbusmenuMenuitem* item,
67 int time,
68 gchar* lens)
69{
70 if (lens != NULL)
71 {
72 ubus_manager_.SendMessage(UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, g_variant_new("(sus)", lens, dash::GOTO_DASH_URI, ""));
73 g_free(lens);
74 }
75}
76
77std::list<DbusmenuMenuitem*> BFBLauncherIcon::GetMenus()
78{
79 std::list<DbusmenuMenuitem*> result;
80 DbusmenuMenuitem* menu_item;
81
82 // Home dash
83 menu_item = dbusmenu_menuitem_new();
84
85 dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Dash Home"));
86 dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
87 dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
88
89 g_signal_connect(menu_item,
90 DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
91 (GCallback)&BFBLauncherIcon::OnMenuitemActivated,
92 g_strdup("home.lens"));
93
94 result.push_back(menu_item);
95
96 // Other lenses..
97 for (auto lens : lenses_.GetLenses())
98 {
99 if (!lens->visible())
100 continue;
101
102 glib::String name(g_markup_escape_text(lens->name().c_str(), -1));
103
104 menu_item = dbusmenu_menuitem_new();
105
106 dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, name.Value());
107 dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
108 dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
109
110 g_signal_connect(menu_item,
111 DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
112 (GCallback)&BFBLauncherIcon::OnMenuitemActivated,
113 g_strdup(lens->id().c_str()));
114
115 result.push_back(menu_item);
116 }
117
118 return result;
119}
120
64} // namespace launcher121} // namespace launcher
65} // namespace unity122} // namespace unity
123
66124
=== 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 @@
2222
23#include "SimpleLauncherIcon.h"23#include "SimpleLauncherIcon.h"
2424
25#include <UnityCore/FilesystemLenses.h>
26
25#include "UBusWrapper.h"27#include "UBusWrapper.h"
2628
27namespace unity29namespace unity
@@ -39,9 +41,16 @@
39 virtual nux::Color GlowColor();41 virtual nux::Color GlowColor();
4042
41 void ActivateLauncherIcon(ActionArg arg);43 void ActivateLauncherIcon(ActionArg arg);
44
45protected:
46 std::list<DbusmenuMenuitem*> GetMenus();
47
42private:48private:
43 unity::UBusManager _ubus_manager;49 static void OnMenuitemActivated(DbusmenuMenuitem* item, int time, gchar* lens);
44 nux::Color _background_color;50
51 static unity::UBusManager ubus_manager_;
52 nux::Color background_color_;
53 dash::FilesystemLenses lenses_;
45};54};
4655
47}56}
4857
=== 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 @@
542{542{
543 glib::String uri;543 glib::String uri;
544 glib::String search_string;544 glib::String search_string;
545 dash::HandledType handled_type;
545546
546 g_variant_get(args, "(sus)", &uri, NULL, &search_string);547 g_variant_get(args, "(sus)", &uri, &handled_type, &search_string);
547548
548 std::string id = AnalyseLensURI(uri.Str());549 std::string id = AnalyseLensURI(uri.Str());
549550
550 home_view_->search_string = "";551 home_view_->search_string = "";
551 lens_bar_->Activate(id);552 lens_bar_->Activate(id);
552553
553554 if ((id == "home.lens" && handled_type != GOTO_DASH_URI ) || !visible_)
554 if (id == "home.lens" || !visible_)
555 ubus_manager_.SendMessage(UBUS_DASH_EXTERNAL_ACTIVATION);555 ubus_manager_.SendMessage(UBUS_DASH_EXTERNAL_ACTIVATION);
556}556}
557557