Merge lp:~aacid/unity/fix_panek_title_escaping_1067357 into lp:unity

[Albert Astals Cid]

Fix escaping of _panel_title

It only has to be escpaed if comes from new_title
if coming from UBUS_LAUNCHER_SELECTION_CHANGED data it is already escaped
Fixes bug #1067357

Fix escaping of _panel_title

It only has to be escpaed if comes from new_title
if coming from UBUS_LAUNCHER_SELECTION_CHANGED data it is already escaped
Fixes bug #1067357

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

89 + const std::string prevTitle = _panel_title;
90 + RefreshTitle();
91 + if (prevTitle == _panel_title && !force && _last_geo == geo && _title_texture)
92 + {
93 + // No need to redraw the title, let's save some CPU time!
94 + return;
95 }

Mh, at this point I think it's better to make RefreshTitle to return a string... Channging it to GetCurrentTitle() so we can:

std::string const& new_title = GetCurrentTitle() // always use not-capital chars on variables

if (new_title == prev_title_) ....

130 + std::string _panel_title;

Please don't make it public, put it back as private.

If you need to access it on testing, just add a: friend class TestPanelMenuView;
Doing so in your struct TestPanelMenuView you can access to this member (see LauncherController or Launcher tests for reference).

+ EXPECT_EQ(panelMenuView._panel_title, "");
Better EXPECT_TRUE( ... .empty()).

264 + EXPECT_TRUE(wm != nullptr);
Use an assert here, so that there won't be crashes:
ASSERT_NE(wm, nullptr);

[Albert Astals Cid (aacid)]

Suggestions integrated

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

Nice, thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://s-jenkins:8080/job/unity-mbs-autolanding/67/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/67/console
    FAILURE: http://s-jenkins:8080/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/67/console

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Reapproving now that the merger is not broken anymore

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/70/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/70/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/70/console

review: Needs Fixing (continuous-integration)

[lp:~aacid/unity/fix_panek_title_escaping_1067357]

2873. By Albert Astals Cid on 2012-10-22

Merge lp:unity

[Albert Astals Cid (aacid)]

Fixed merge conflict

=== modified file 'panel/PanelMenuView.cpp'
--- panel/PanelMenuView.cpp	2012-10-11 01:44:15 +0000
+++ panel/PanelMenuView.cpp	2012-10-22 09:33:20 +0000
@@ -785,6 +785,46 @@
   gtk_style_context_restore(style_context);
 }
 
+std::string PanelMenuView::GetCurrentTitle() const
+{
+  if (!_switcher_showing && !_launcher_keynav)
+  {
+    WindowManager& wm = WindowManager::Default();
+    std::string new_title;
+
+    if (wm.IsScaleActive())
+    {
+      if (wm.IsScaleActiveForGroup())
+        new_title = GetActiveViewName(true);
+      else if (_we_control_active)
+        new_title = _desktop_name;
+    }
+    else if (wm.IsExpoActive())
+    {
+      new_title = _desktop_name;
+    }
+    else if (!_we_control_active)
+    {
+      new_title = "";
+    }
+    else
+    {
+      new_title = GetActiveViewName();
+      _window_buttons->SetControlledWindow(_active_xid);
+    }
+
+    // _panel_title needs to be only escaped when computed
+    // in this function, if it comes from OnLauncherSelectionChanged
+    // it is already escaped
+    glib::String escaped(g_markup_escape_text(new_title.c_str(), -1));
+    return escaped.Str();
+  }
+  else
+  {
+    return _panel_title;
+  }
+}
+
 void PanelMenuView::Refresh(bool force)
 {
   nux::Geometry const& geo = GetGeometry();
@@ -794,42 +834,13 @@
   if (geo.width > _monitor_geo.width)
     return;
 
-  WindowManager& wm = WindowManager::Default();
-  std::string new_title;
-
-  if (wm.IsScaleActive())
-  {
-    if (wm.IsScaleActiveForGroup())
-      new_title = GetActiveViewName(true);
-    else if (_we_control_active)
-      new_title = _desktop_name;
-  }
-  else if (wm.IsExpoActive())
-  {
-    new_title = _desktop_name;
-  }
-  else if (!_we_control_active)
-  {
-    new_title = "";
-  }
-  else if (!_switcher_showing && !_launcher_keynav)
-  {
-    new_title = GetActiveViewName();
-    _window_buttons->SetControlledWindow(_active_xid);
-  }
-
-  if (!_switcher_showing && !_launcher_keynav)
-  {
-    if (_panel_title != new_title)
-    {
-      _panel_title = new_title;
-    }
-    else if (!force && _last_geo == geo && _title_texture)
-    {
-      // No need to redraw the title, let's save some CPU time!
-      return;
-    }
-  }
+  const std::string& new_title = GetCurrentTitle();
+  if (new_title == _panel_title && !force && _last_geo == geo && _title_texture)
+  {
+    // No need to redraw the title, let's save some CPU time!
+    return;
+  }
+  _panel_title = new_title;
 
   if (_panel_title.empty())
   {
@@ -843,12 +854,7 @@
   cairo_set_operator(cr, CAIRO_OPERATOR_CLEAR);
   cairo_paint(cr);
 
-  glib::String escaped(g_markup_escape_text(_panel_title.c_str(), -1));
-
-  std::ostringstream bold_label;
-  bold_label << "<b>" << escaped.Str() << "</b>";
-
-  DrawTitle(cr, geo, bold_label.str());
+  DrawTitle(cr, geo, _panel_title);
 
   cairo_destroy(cr);
 

=== modified file 'panel/PanelMenuView.h'
--- panel/PanelMenuView.h	2012-10-11 01:44:15 +0000
+++ panel/PanelMenuView.h	2012-10-22 09:33:20 +0000
@@ -68,8 +68,11 @@
   virtual nux::Area* FindAreaUnderMouse(const nux::Point& mouse_position,
                                         nux::NuxEventType event_type);
   virtual void OnEntryAdded(indicator::Entry::Ptr const& entry);
+  virtual std::string GetActiveViewName(bool use_appname = false) const;
 
 private:
+  friend class TestPanelMenuView;
+
   void OnActiveChanged(PanelIndicatorEntryView* view, bool is_active);
   void OnViewOpened(BamfMatcher* matcher, BamfView* view);
   void OnViewClosed(BamfMatcher* matcher, BamfView* view);
@@ -100,6 +103,7 @@
   void OnMaximizedGrabEnd(int x, int y);
 
   void FullRedraw();
+  std::string GetCurrentTitle() const;
   void Refresh(bool force = false);
   void DrawTitle(cairo_t *cr_real, nux::Geometry const& geo, std::string const& label) const;
 
@@ -109,8 +113,6 @@
 
   BamfWindow* GetBamfWindowForXid(Window xid) const;
 
-  std::string GetActiveViewName(bool use_appname = false) const;
-
   void OnSwitcherShown(GVariant* data);
   void OnLauncherKeyNavStarted(GVariant* data);
   void OnLauncherKeyNavEnded(GVariant* data);

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-10-19 14:37:49 +0000
+++ tests/CMakeLists.txt	2012-10-22 09:33:20 +0000
@@ -200,6 +200,7 @@
                  test_launcher_icon.cpp
                  test_keyboard_util.cpp
                  test_panel_style.cpp
+                 test_panel_menu_view.cpp
                  test_places_group.cpp
                  test_previews_application.cpp
                  test_previews_generic.cpp
@@ -226,7 +227,7 @@
                  gmockvolume.c
                  ${CMAKE_SOURCE_DIR}/plugins/unityshell/src/WindowMinimizeSpeedController.cpp
                 )
-  target_link_libraries(test-gtest gtest gmock unity-shared ${LIBS} launcher-lib unity-shared-standalone shortcuts-lib previews-lib hud-lib switcher-lib dash-lib)
+  target_link_libraries(test-gtest gtest gmock unity-shared ${LIBS} launcher-lib unity-shared-standalone shortcuts-lib previews-lib hud-lib switcher-lib dash-lib panel-lib)
   add_test(UnityGTest test-gtest)
   add_dependencies(test-gtest unity-core-${UNITY_API_VERSION} gtest gmock)
 

=== added file 'tests/test_panel_menu_view.cpp'
--- tests/test_panel_menu_view.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_panel_menu_view.cpp	2012-10-22 09:33:20 +0000
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ */
+
+#include <gmock/gmock.h>
+
+#include <Nux/Nux.h>
+#include "PanelMenuView.h"
+#include "PanelStyle.h"
+#include "UnitySettings.h"
+#include "UBusMessages.h"
+#include "StandaloneWindowManager.h"
+#include "test_utils.h"
+
+using namespace testing;
+
+namespace unity
+{
+
+class MockPanelMenuView : public PanelMenuView
+{
+  protected:
+    virtual std::string GetActiveViewName(bool use_appname) const
+    {
+      return "<>'";
+    }
+};
+
+struct TestPanelMenuView : public testing::Test
+{
+  virtual void SetUp()
+  {
+  }
+
+  void ProcessUBusMessages()
+  {
+    bool expired = false;
+    glib::Idle idle([&] { expired = true; return false; },
+                    glib::Source::Priority::LOW);
+    Utils::WaitUntil(expired);
+  }
+
+  std::string panelTitle() const
+  {
+    const PanelMenuView *panel = &panelMenuView;
+    return panel->GetCurrentTitle();
+  }
+
+private:
+  // The order is important, i.e. PanelMenuView needs
+  // panel::Style that needs Settings
+  Settings settings;
+  panel::Style panelStyle;
+  MockPanelMenuView panelMenuView;
+};
+
+TEST_F(TestPanelMenuView, Escaping)
+{
+  static const char *escapedText = "Panel d&amp;Inici";
+  EXPECT_TRUE(panelTitle().empty());
+
+  UBusManager ubus;
+  ubus.SendMessage(UBUS_LAUNCHER_START_KEY_NAV, NULL);
+  ubus.SendMessage(UBUS_LAUNCHER_SELECTION_CHANGED,
+                   g_variant_new_string(escapedText));
+  ProcessUBusMessages();
+
+  EXPECT_EQ(panelTitle(), escapedText);
+
+  ubus.SendMessage(UBUS_LAUNCHER_END_KEY_NAV, NULL);
+  ProcessUBusMessages();
+
+  StandaloneWindowManager *wm = dynamic_cast<StandaloneWindowManager *>(&WindowManager::Default());
+  ASSERT_NE(wm, nullptr);
+  // Change the wm to trick PanelMenuView::RefreshTitle to call GetActiveViewName
+  wm->SetScaleActive(true);
+  wm->SetScaleActiveForGroup(true);
+
+  EXPECT_EQ(panelTitle(), "&lt;&gt;&apos;");
+}
+
+}

=== modified file 'unity-shared/StandaloneWindowManager.cpp'
--- unity-shared/StandaloneWindowManager.cpp	2012-10-11 01:44:15 +0000
+++ unity-shared/StandaloneWindowManager.cpp	2012-10-22 09:33:20 +0000
@@ -44,6 +44,8 @@
 StandaloneWindowManager::StandaloneWindowManager()
   : expo_state_(false)
   , in_show_desktop_(false)
+  , scale_active_(false)
+  , scale_active_for_group_(false)
 {
 }
 
@@ -136,14 +138,24 @@
 void StandaloneWindowManager::TerminateScale()
 {}
 
+void StandaloneWindowManager::SetScaleActive(bool scale_active)
+{
+  scale_active_ = scale_active;
+}
+
 bool StandaloneWindowManager::IsScaleActive() const
 {
-  return false;
+  return scale_active_;
+}
+
+void StandaloneWindowManager::SetScaleActiveForGroup(bool scale_active_for_group)
+{
+  scale_active_for_group_ = scale_active_for_group;
 }
 
 bool StandaloneWindowManager::IsScaleActiveForGroup() const
 {
-  return false;
+  return scale_active_for_group_;
 }
 
 void StandaloneWindowManager::InitiateExpo()

=== modified file 'unity-shared/StandaloneWindowManager.h'
--- unity-shared/StandaloneWindowManager.h	2012-10-04 07:10:39 +0000
+++ unity-shared/StandaloneWindowManager.h	2012-10-22 09:33:20 +0000
@@ -95,12 +95,18 @@
 
   virtual std::string GetWindowName(Window window_id) const;
 
+  // Mock functions
+  void SetScaleActive(bool scale_active);
+  void SetScaleActiveForGroup(bool scale_active_for_group);
+
 protected:
   virtual void AddProperties(GVariantBuilder* builder);
 
 private:
   bool expo_state_;
   bool in_show_desktop_;
+  bool scale_active_;
+  bool scale_active_for_group_;
 };
 
 }