mvf / qolibri

Continuation of the qolibri EPWING dictionary/book reader
GNU General Public License v2.0
172 stars 13 forks source link

search gives "No results" for a certain word #41

Closed omatsuda closed 3 years ago

omatsuda commented 3 years ago

Thank you very much for this nice software.

Everything works just fine, except one remaining strange behavior. When I search the word "avoid", qolibri just returns 'No rusults for "avoid".' It happens for either "Exact word search" or "Forward search". If I search "avoi", it gives the results for avoid, avoidable, .. etc.

Backword search for "avoid" gives the correct results.

Could anybody help me to solve this?

I'm using several dictionaries including Longman 4th Edition, and some English-Japanese dictionalies.

The program itself has been compiled on FreeBSD 12.2-Release. (For the build, I had to add a line link_directories(/usr/lib /usr/local/lib) just below the line include(GNUInstallDirs) in CMakeLists.txt. Otherwise the build fails at the linker stage to find libeb.so which is installed at /usr/local/lib in FreeBSD.)

darlopvil commented 3 years ago

Same here. I'm using Windows 10 and literally, you can't search any word.

mvf commented 3 years ago

Confirmed, thanks for reporting this! That's indeed a pretty nasty bug. It's strange that it works on 1.x, things shouldn't have changed much in and around the EB library.

Also, thanks for reporting the linker path issue. This is probably because we (sloppily) just link_libraries the eb lib without doing find_library first. This works as long as eb is installed in /usr (as is typical on Linux), but it's technically incorrect (everywhere) because it bypasses CMAKE_SYSTEM_PREFIX_PATH.

mvf commented 3 years ago

Quick workaround in src/searchpage.cpp:

RET_SEARCH SearchPage::search(const Query& query)
     ...
     switch (query.method.direction) {
     case ExactWordSearch:
     case ForwardSearch:
-        queries << stemWords(query.query);
         break;
     default:
         break;

Turns out that since 2010 with change f01a37cf71e829d9bb603855e671356b419e2442, qolibri tries to deinflect English words in Exact and Forward searches, e.g. "avoids" will do an implicit second query for "avoid". This bug presents only when the last one of these pseudo-deinflections is not found. For "avoid" that's "avoy", which doesn't exist → "No results". For "vices" it's "vex", so it works, even though the words are unrelated. "house" doesn't pseudo-deinflect, so it's unaffected.

The bug is that when looping over the queries for the individual words, the outcome of the last search determines the overall outcome:

RET_SEARCH SearchPage::doSearch(const Query& query, PageItems &items, int &itemIndex)
{
    ...
    foreach (const QString &q, queries) {
        retStatus = doSearch(Query(q, query.method), items, itemIndex);
    }

This worked at the time because in 2009 15dd5b3da371e58f4dd758a9f4201290f6fc589b had changed what is now SearchPage::doSearch to succeed, even when there were no hits. This changed in 2012 with 6eb478726ac161c679dfcb4f5b698500eaa37c97, but the regression went unnoticed until now, so thanks again for reporting!

omatsuda commented 3 years ago

Thank you very much! The suggested workaround solved the issue perfectly.

darlopvil commented 3 years ago

Hi, I would like to know what a Windows 10 user could do to with the workaround. It seems I cannot use the program until a new version gets released.

omatsuda commented 3 years ago

In FreeBSD, we have "ports collection" which offers a simple way to compile and install third party applications. I have prepared a port for qolibri and now it is open to the public. I included the workaround patch suggested by Matthias in this port. But now I'm worrying that I should have got a permission to include this before the port is open. I'm sorry about this, and if you don't like this, please tell me.

For CMakeList.txt, I was suggested by the FreeBSD team the following patch to make it more generic.

--- CMakeLists.txt.orig 2020-02-28 16:02:25 UTC
+++ CMakeLists.txt
@@ -16,6 +16,8 @@ set(CMAKE_AUTOUIC ON)
 set(CMAKE_AUTORCC ON)

 find_package(Qt5 COMPONENTS LinguistTools Multimedia Network WebEngine WebEngineWidgets Widgets REQUIRED)
+find_library(EB_LIBRARY eb)
+find_library(Z_LIBRARY z)

 add_executable(qolibri MACOSX_BUNDLE WIN32
     images/qolibri.icns
@@ -154,6 +156,6 @@ set_source_files_properties(${TS_FILES} PROPERTIES OUT
 qt5_add_translation(QM_FILES ${TS_FILES})
 target_sources(qolibri PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/translations/translations.qrc" ${QM_FILES} ${TS_FILES})

-target_link_libraries(qolibri Qt5::Multimedia Qt5::Network Qt5::WebEngine Qt5::WebEngineWidgets Qt5::Widgets eb z)
+target_link_libraries(qolibri Qt5::Multimedia Qt5::Network Qt5::WebEngine Qt5::WebEngineWidgets Qt5::Widgets ${EB_LIBRARY} ${Z_LIBRARY})

 install(TARGETS qolibri DESTINATION "${CMAKE_INSTALL_BINDIR}")
mvf commented 3 years ago

@omatsuda Thanks so much for bringing qolibri to FreeBSD! Of course, shipping the workaround to your users is more than fine. It's me who should apologize for the lazy CMakeLists. :wink: If you don't mind, I'll push the patch you suggested.

@darlopvil Yes, you'd have to compile yourself. I think we're about ready to release 2.1.4 though.

omatsuda commented 3 years ago

Thank you! And I would be happy if the patch for CMakeLists.txt is adopted in the next release.