SYNERGY-1307 Language reading on linux (#7114)

* SYNERGY-1307 Test implementation

* Update CMakeLists.txt

* SYNERGY-1307 Code cleanup

* SYNERGY-1307 Fix code smell

* SYNERGY-1307 C++11 style of initialisation

* Update ChangeLog

* Fix code smells

* SYNERGY-1307 Fix code smells

* Fix Ubuntu16 compilation

* SYNERGY-1307 Fix windows build

* SYNERGY-1307 Fix code smell

* SYNERGY-1307 Fix macOS build

* SYNERGY-1307 Fix Linux build

* SYNERGY-1307 Text for notification has been changed as requested

* SYNERGY-1307 Update notification text and fix union initialisation

CMakeLists.txt

@@ -244,11 +244,15 @@ if (UNIX)
 
         endif()
 
+        if (!X11_xkbfile_FOUND)
+            message (FATAL_ERROR "Missing library: xkbfile")
+        endif()
+
         if (HAVE_Xtst)
 
             # Xtxt depends on X11.
             set (HAVE_X11)
-            list (APPEND libs Xtst X11)
+            list (APPEND libs Xtst X11 xkbfile)
 
         else()
 

ChangeLog

@@ -17,6 +17,7 @@ Bug fixes:
 - #7100 | #7101 No configuration available on Windows
 - #7097 The title "Enterprise" disappeares after clicking on "Preferences"
 - #7108 Wrong characters on client for unicode
+- #7114 Fixed problem with reading languages on Linux system
 
 Enhancements:
 - #7068 Add Synergy restart when settings changed

src/lib/client/ServerProxy.cpp

@@ -975,7 +975,7 @@ ServerProxy::setActiveServerLanguage(const String& language)
 
         if (!m_languageManager.isLanguageInstalled(m_serverLanguage)) {
             if(!m_isUserNotifiedAboutLanguageSyncError) {
-                AppUtil::instance().showNotification("Language error", "Current server language is not installed on client!");
+                AppUtil::instance().showNotification("Language error", "Current server language is not installed on client.");
                 m_isUserNotifiedAboutLanguageSyncError = true;
             }
         }
@@ -994,7 +994,7 @@ ServerProxy::checkMissedLanguages() const
     auto missedLanguages = m_languageManager.getMissedLanguages();
     if (!missedLanguages.empty()) {
         AppUtil::instance().showNotification("Language synchronization error",
-              "You need to install these languages on this computer to enable support for multiple languages: "
+              "You need to install these languages on this computer and restart Synergy to enable support for multiple languages: "
               + missedLanguages);
     }
 }

src/lib/synergy/KeyMap.cpp

@@ -1348,7 +1348,7 @@ KeyMap::KeyItem::operator==(const KeyItem& x) const
 
 KeyMap::Keystroke::Keystroke(KeyButton button,
                 bool press, bool repeat, UInt32 data) :
-    m_type(kButton), m_data{}
+    m_type(kButton)
 {
     m_data.m_button.m_button = button;
     m_data.m_button.m_press = press;
@@ -1357,7 +1357,7 @@ KeyMap::Keystroke::Keystroke(KeyButton button,
 }
 
 KeyMap::Keystroke::Keystroke(SInt32 group, bool absolute, bool restore) :
-    m_type(kGroup), m_data{}
+    m_type(kGroup)
 {
     m_data.m_group.m_group = group;
     m_data.m_group.m_absolute = absolute;

src/lib/synergy/KeyMap.h

@@ -103,17 +103,12 @@ public:
 		};
 		union Data {
 		public:
-			Data() = default;
+			Button		m_button;
-			Data(Data &&) = default;
-			Data(const Data&) = default;
-			Data& operator=(const Data&) = default;
-
-			Button		m_button {};
 			Group		m_group;
 		};
 
 		EType			m_type {};
-		Data			m_data;
+		Data			m_data {};
 	};
 
 	//! A sequence of keystrokes

src/lib/synergy/unix/AppUtilUnix.cpp

@@ -21,7 +21,7 @@
 #include <thread>
 
 #if WINAPI_XWINDOWS
-#include "synergy/X11LayoutsParser.h"
+#include "synergy/unix/X11LayoutsParser.h"
 #include <X11/XKBlib.h>
 #elif WINAPI_CARBON
 #include <Carbon/Carbon.h>
@@ -67,7 +67,7 @@ AppUtilUnix::getKeyboardLayoutList()
     std::vector<String> layoutLangCodes;
 
 #if WINAPI_XWINDOWS
-    layoutLangCodes = X11LayoutsParser::getX11LanguageList("/etc/default/keyboard", "/usr/share/X11/xkb/rules/evdev.xml");
+    layoutLangCodes = X11LayoutsParser::getX11LanguageList("/usr/share/X11/xkb/rules/evdev.xml");
 #elif WINAPI_CARBON
     CFStringRef keys[] = { kTISPropertyInputSourceCategory };
     CFStringRef values[] = { kTISCategoryKeyboardInputSource };

src/lib/synergy/unix/SynergyXkbKeyboard.cpp

@@ -0,0 +1,67 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2012-2021 Symless Ltd.
+ * Copyright (C) 2002 Chris Schoeneman
+ *
+ * This package is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * found in the file LICENSE that should have accompanied this file.
+ *
+ * This package is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#if WINAPI_XWINDOWS
+#include <memory>
+
+#include "base/Log.h"
+#include "SynergyXkbKeyboard.h"
+
+namespace synergy {
+
+namespace linux {
+
+SynergyXkbKeyboard::SynergyXkbKeyboard()
+{
+    using XkbDisplay = std::unique_ptr<Display, decltype(&XCloseDisplay)>;
+    XkbDisplay display(XkbOpenDisplay(nullptr, nullptr, nullptr, nullptr, nullptr, nullptr), &XCloseDisplay);
+
+    if (display) {
+        if (!XkbRF_GetNamesProp(display.get(), nullptr, &m_data)) {
+            LOG((CLOG_WARN "Error reading keyboard layouts"));
+        }
+    }
+    else {
+        LOG((CLOG_WARN "Can't open Xkb diaplay during reading languages"));
+    }
+}
+
+const char* SynergyXkbKeyboard::getLayout() const
+{
+    return m_data.layout ? m_data.layout : "us";
+}
+
+const char* SynergyXkbKeyboard::getVariant() const
+{
+    return m_data.variant ? m_data.variant : "";
+}
+
+SynergyXkbKeyboard::~SynergyXkbKeyboard()
+{
+    std::free(m_data.model);
+    std::free(m_data.layout);
+    std::free(m_data.variant);
+    std::free(m_data.options);
+}
+
+
+} //namespace Unix
+
+} //namespace synergy
+
+#endif //WINAPI_XWINDOWS

src/lib/synergy/unix/SynergyXkbKeyboard.h

@@ -0,0 +1,55 @@
+/*
+ * synergy -- mouse and keyboard sharing utility
+ * Copyright (C) 2012-2021 Symless Ltd.
+ * Copyright (C) 2002 Chris Schoeneman
+ *
+ * This package is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * found in the file LICENSE that should have accompanied this file.
+ *
+ * This package is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#if WINAPI_XWINDOWS
+
+#ifndef XKBKEYBOARD_H
+#define XKBKEYBOARD_H
+
+
+#include <cstdio>
+#include <X11/XKBlib.h>
+#include <X11/extensions/XKBrules.h>
+
+namespace synergy {
+
+namespace linux {
+
+class SynergyXkbKeyboard
+{
+    XkbRF_VarDefsRec m_data = {};
+
+public:
+    SynergyXkbKeyboard();
+    SynergyXkbKeyboard(const SynergyXkbKeyboard&) = delete;
+    SynergyXkbKeyboard& operator=(const SynergyXkbKeyboard&) = delete;
+
+    const char* getLayout() const;
+    const char* getVariant() const;
+
+    ~SynergyXkbKeyboard();
+};
+
+} //namespace Unix
+
+} //namespace synergy
+
+#endif // XKBKEYBOARD_H
+
+#endif //WINAPI_XWINDOWS
+
+

src/lib/synergy/unix/X11LayoutsParser.cpp

@@ -15,15 +15,31 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-
+#if WINAPI_XWINDOWS
 #include <fstream>
 #include <sstream>
 #include <algorithm>
 
 #include "base/Log.h"
-#include "synergy/X11LayoutsParser.h"
+#include "X11LayoutsParser.h"
 #include "ISO639Table.h"
 #include "pugixml.hpp"
+#include "SynergyXkbKeyboard.h"
+
+namespace
+{
+
+void splitLine(std::vector<String>& parts, const String& line, char delimiter)
+{
+    std::stringstream stream(line);
+    while (stream.good()) {
+        String part;
+        getline(stream, part, delimiter);
+        parts.push_back(part);
+    }
+}
+
+} //namespace
 
 bool
 X11LayoutsParser::readXMLConfigItemElem(const pugi::xml_node* root, std::vector<Lang>& langList)
@@ -100,8 +116,8 @@ X11LayoutsParser::appendVectorUniq(const std::vector<String>& source, std::vecto
 void
 X11LayoutsParser::convertLayoutToISO639_2(const String&        pathToEvdevFile,
                                           bool                 needToReloadEvdev,
-                                          std::vector<String>  layoutNames,
+                                          const std::vector<String>&  layoutNames,
-                                          std::vector<String>  layoutVariantNames,
+                                          const std::vector<String>&  layoutVariantNames,
                                           std::vector<String>& iso639_2Codes)
 {
     if(layoutNames.size() != layoutVariantNames.size()) {
@@ -114,7 +130,8 @@ X11LayoutsParser::convertLayoutToISO639_2(const String&        pathToEvdevFile,
         allLang = getAllLanguageData(pathToEvdevFile);
     }
     for (size_t i = 0; i < layoutNames.size(); i++) {
-        auto langIter = std::find_if(allLang.begin(), allLang.end(), [n=layoutNames[i]](const Lang& l) {return l.name == n;});
+        const auto& layoutName = layoutNames[i];
+        auto langIter = std::find_if(allLang.begin(), allLang.end(), [&layoutName](const Lang& l) {return l.name == layoutName;});
         if(langIter == allLang.end()) {
             LOG((CLOG_WARN "Language \"%s\" is unknown", layoutNames[i].c_str()));
             continue;
@@ -125,8 +142,9 @@ X11LayoutsParser::convertLayoutToISO639_2(const String&        pathToEvdevFile,
             toCopy = &langIter->layoutBaseISO639_2;
         }
         else {
+            const auto& variantName = layoutVariantNames[i];
             auto langVariantIter = std::find_if(langIter->variants.begin(), langIter->variants.end(),
-                                                [n=layoutVariantNames[i]](const Lang& l) {return l.name == n;});
+                                                [&variantName](const Lang& l) {return l.name == variantName;});
             if(langVariantIter == langIter->variants.end()) {
                 LOG((CLOG_WARN "Variant \"%s\" of language \"%s\" is unknown", layoutVariantNames[i].c_str(), layoutNames[i].c_str()));
                 continue;
@@ -147,13 +165,17 @@ X11LayoutsParser::convertLayoutToISO639_2(const String&        pathToEvdevFile,
 }
 
 std::vector<String>
-X11LayoutsParser::getX11LanguageList(const String& pathToKeyboardFile, const String& pathToEvdevFile)
+X11LayoutsParser::getX11LanguageList(const String& pathToEvdevFile)
 {
     std::vector<String> layoutNames;
     std::vector<String> layoutVariantNames;
-    std::vector<String> iso639_2Codes;
 
-    parseKeyboardFile(pathToKeyboardFile, layoutNames, layoutVariantNames);
+    synergy::linux::SynergyXkbKeyboard keyboard;
+    splitLine(layoutNames, keyboard.getLayout(), ',');
+    splitLine(layoutVariantNames, keyboard.getVariant(), ',');
+
+    std::vector<String> iso639_2Codes;
+    iso639_2Codes.reserve(layoutNames.size());
     convertLayoutToISO639_2(pathToEvdevFile, true, layoutNames, layoutVariantNames, iso639_2Codes);
     return convertISO639_2ToISO639_1(iso639_2Codes);
 }
@@ -177,43 +199,6 @@ X11LayoutsParser::convertLayotToISO(const String& pathToEvdevFile, const String&
     return *iso639_1Codes.begin();
 }
 
-void
-X11LayoutsParser::parseKeyboardFile(const String&        pathToKeyboardFile,
-                                    std::vector<String>& layoutNames,
-                                    std::vector<String>& layoutVariantNames)
-{
-    layoutNames.clear();
-    layoutVariantNames.clear();
-
-    std::ifstream file(pathToKeyboardFile);
-    if (!file.is_open()) {
-        LOG((CLOG_WARN "x11 keyboard layouts file \"%s\" is missed.", pathToKeyboardFile.c_str()));
-        return;
-    }
-
-    const String layoutLinePrefix = "XKBLAYOUT=";
-    const String variantLinePrefix = "XKBVARIANT=";
-
-    auto splitLine = [](std::vector<String>& splitted, const String& line, char delim) {
-        std::stringstream ss(line);
-        String code;
-        while(ss.good()) {
-            getline(ss, code, delim);
-            splitted.push_back(code);
-        }
-    };
-
-    String line;
-    while (std::getline(file, line)) {
-        if (line.rfind(layoutLinePrefix, 0) == 0) {
-            splitLine(layoutNames, line.substr(layoutLinePrefix.size()), ',');
-        }
-        else if (line.rfind(variantLinePrefix, 0) == 0) {
-            splitLine(layoutVariantNames, line.substr(variantLinePrefix.size()), ',');
-        }
-    }
-}
-
 std::vector<String>
 X11LayoutsParser::convertISO639_2ToISO639_1(const std::vector<String>& iso639_2Codes)
 {
@@ -231,3 +216,5 @@ X11LayoutsParser::convertISO639_2ToISO639_1(const std::vector<String>& iso639_2C
 
     return result;
 }
+
+#endif //WINAPI_XWINDOWS

src/lib/synergy/unix/X11LayoutsParser.h

@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#if WINAPI_XWINDOWS
 #pragma once
 #include "base/String.h"
 
@@ -26,7 +27,7 @@ namespace pugi
 
 class X11LayoutsParser {
 public:
-    static std::vector<String> getX11LanguageList(const String& pathToKeyboardFile, const String& pathToEvdevFile);
+    static std::vector<String> getX11LanguageList(const String& pathToEvdevFile);
     static String              convertLayotToISO(const String& pathToEvdevFile, const String& layoutLangCode, bool needToReloadFiles = false);
 
 private:
@@ -46,13 +47,11 @@ private:
 
     static void                convertLayoutToISO639_2(const String&        pathToEvdevFile,
                                                        bool                 needToReloadEvdev,
-                                                       std::vector<String>  layoutNames,
+                                                       const std::vector<String>& layoutNames,
-                                                       std::vector<String>  layoutVariantNames,
+                                                       const std::vector<String>& layoutVariantNames,
                                                        std::vector<String>& iso639_2Codes);
 
-    static void                parseKeyboardFile(const String&        pathToKeyboardFile,
-                                                 std::vector<String>& layoutNames,
-                                                 std::vector<String>& layoutVariantNames);
-
     static std::vector<String> convertISO639_2ToISO639_1(const std::vector<String>& iso639_2Codes);
 };
+
+#endif //WINAPI_XWINDOWS

src/test/unittests/synergy/X11LayoutParserTests.cpp

@@ -15,60 +15,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "synergy/X11LayoutsParser.h"
+#if WINAPI_XWINDOWS
+#include "synergy/unix/X11LayoutsParser.h"
 #include "test/global/gtest.h"
 #include <fstream>
 
 void createTestFiles()
 {
-    std::ofstream correctKeyboardFile ("correctKeyboard");
-    if(!correctKeyboardFile.is_open()) {
-        FAIL();
-    }
-
-    correctKeyboardFile << "XKBLAYOUT=us,ru" << std::endl;
-    correctKeyboardFile << "XKBVARIANT=eng," << std::endl;
-    correctKeyboardFile << "BACKSPACE=guess" << std::endl;
-    correctKeyboardFile.close();
-
-    std::ofstream keyboardFromFutureFile ("keyboardFromFuture");
-    if(!keyboardFromFutureFile.is_open()) {
-        FAIL();
-    }
-
-    keyboardFromFutureFile << "XKBLAYOUT=futureLangName" << std::endl;
-    keyboardFromFutureFile << "XKBVARIANT=" << std::endl;
-    keyboardFromFutureFile.close();
-
-    std::ofstream incorrectKeyboardFile1 ("incorrectKeyboard1");
-    if(!incorrectKeyboardFile1.is_open()) {
-        FAIL();
-    }
-
-    incorrectKeyboardFile1 << "XKBLAYOUT=unknownLangCode" << std::endl;
-    incorrectKeyboardFile1 << "XKBVARIANT=eng" << std::endl;
-    incorrectKeyboardFile1 << "BACKSPACE=guess" << std::endl;
-    incorrectKeyboardFile1.close();
-
-    std::ofstream incorrectKeyboardFile2 ("incorrectKeyboard2");
-    if(!incorrectKeyboardFile2.is_open()) {
-        FAIL();
-    }
-
-    incorrectKeyboardFile2 << "XKBLAYOUT=us" << std::endl;
-    incorrectKeyboardFile2 << "XKBVARIANT=unknownLangVariant" << std::endl;
-    incorrectKeyboardFile2 << "BACKSPACE=guess" << std::endl;
-    incorrectKeyboardFile2.close();
-
-    std::ofstream incorrectKeyboardFile3 ("incorrectKeyboard3");
-    if(!incorrectKeyboardFile3.is_open()) {
-        FAIL();
-    }
-
-    incorrectKeyboardFile3 << "XKBLAYOUT=us,ru,by" << std::endl;
-    incorrectKeyboardFile3 << "XKBVARIANT=," << std::endl;
-    incorrectKeyboardFile3.close();
-
     std::ofstream correctEvdevFile ("correctEvdev.xml");
     if(!correctEvdevFile.is_open()) {
         FAIL();
@@ -172,44 +125,25 @@ TEST(X11LayoutsParsingTests, xmlCorrectParsingTest)
 {
     createTestFiles();
     std::vector<String> expectedResult = { "en", "ru" };
-    auto parsedResult = X11LayoutsParser::getX11LanguageList("correctKeyboard", "correctEvdev.xml");
+    auto parsedResult = X11LayoutsParser::getX11LanguageList("correctEvdev.xml");
 
     EXPECT_EQ(parsedResult, parsedResult);
 }
 
-TEST(X11LayoutsParsingTests, xmlParsingMissedKeyboardFileTest)
-{
-    auto parsedResult = X11LayoutsParser::getX11LanguageList("missedFile", "correctEvdev.xml");
-    EXPECT_TRUE(parsedResult.empty());
-}
-
 TEST(X11LayoutsParsingTests, xmlParsingMissedEvdevFileTest)
 {
-    auto parsedResult = X11LayoutsParser::getX11LanguageList("correctKeyboard", "missedFile");
+    auto parsedResult = X11LayoutsParser::getX11LanguageList("missedFile");
     EXPECT_TRUE(parsedResult.empty());
 }
 
 TEST(X11LayoutsParsingTests, xmlParsingIncorrectEvdevFileTest)
 {
     std::vector<String> parsedResult;
-    parsedResult = X11LayoutsParser::getX11LanguageList("correctKeyboard", "incorrectEvdev1.xml");
+    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectEvdev1.xml");
     EXPECT_TRUE(parsedResult.empty());
-    parsedResult = X11LayoutsParser::getX11LanguageList("correctKeyboard", "incorrectEvdev2.xml");
+    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectEvdev2.xml");
     EXPECT_TRUE(parsedResult.empty());
-    parsedResult = X11LayoutsParser::getX11LanguageList("correctKeyboard", "incorrectEvdev3.xml");
+    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectEvdev3.xml");
-    EXPECT_TRUE(parsedResult.empty());
-}
-
-TEST(X11LayoutsParsingTests, xmlParsingIncorrectKeyboardFileTest)
-{
-    std::vector<String> parsedResult;
-    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectKeyboard1", "correctEvdev.xml");
-    EXPECT_TRUE(parsedResult.empty());
-    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectKeyboard2", "correctEvdev.xml");
-    EXPECT_TRUE(parsedResult.empty());
-    parsedResult = X11LayoutsParser::getX11LanguageList("incorrectKeyboard3", "correctEvdev.xml");
-    EXPECT_TRUE(parsedResult.empty());
-    parsedResult = X11LayoutsParser::getX11LanguageList("keyboardFromFuture", "evdevFromFuture.xml");
     EXPECT_TRUE(parsedResult.empty());
 }
 
@@ -219,3 +153,5 @@ TEST(X11LayoutsParsingTests, layoutConvertTest)
     EXPECT_EQ(X11LayoutsParser::convertLayotToISO("incorrectEvdev1.xml", "us", true), "");
     EXPECT_EQ(X11LayoutsParser::convertLayotToISO("evdevFromFuture.xml", "us", true), "");
 }
+
+#endif