Bug 3985 - SimpleWelcome translates KDE places (bookmarks)
: SimpleWelcome translates KDE places (bookmarks)
Status: RESOLVED FIXED
Product: Desktop Bugs
Classification: ROSA Desktop
Component: Main Packages
: unspecified
: All Linux
: Normal normal
: ---
Assigned To: ROSA Linux Bugs
: ROSA Linux Bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-04-19 23:33 MSD by Dmitry
Modified: 2014-05-06 17:48 MSD (History)
4 users (show)

See Also:
RPM Package:
ISO-related:
Bad POT generating:
Upstream:
vladimir.potapov: qa_verified+
alex.burmashev: published+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry 2014-04-19 23:33:59 MSD
Description of problem:
SimpleWelcome translates bookmarks named in English.

Version-Release number of selected component (if applicable):


How reproducible:
Always

Steps to Reproduce:
1. Create bookmark "Music"
2. See that Dolphin shows bookmark as "Music"
3. See that SimpleWelcome shows the bookmark as "Музыка"
Comment 1 Andrey Bondrov 2014-04-20 20:42:03 MSD
Close Dolphin, then start it again. You'll see "Музыка" instead of Music. That's how KDE works. You can either translate bookmarks or not.
Comment 2 Andrey Bondrov 2014-04-21 08:30:08 MSD
New build list:

https://abf.rosalinux.ru/build_lists/1799569
https://abf.rosalinux.ru/build_lists/1799570

This build doesn't resolve the issue, just adds context where to look for translations:

- newItem["caption"] = i18n(bm.fullText().toUtf8());
+ newItem["caption"] = ki18nc("KFile System Bookmarks",bm.fullText().toUtf8()).toString();

I suggest to approve these builds ASAP.
Comment 3 Dmitry 2014-04-21 09:54:05 MSD
-1, I would prefer don't apply that patch because:
1) that is really bad code
2) an application should never translate string that is set by the user
Comment 4 Andrey Bondrov 2014-04-21 10:41:27 MSD
(In reply to comment #3)
> -1, I would prefer don't apply that patch because:
> 1) that is really bad code

Surely it's a workaround, not a final solution. But is there anything better?

> 2) an application should never translate string that is set by the user

That requires re-work of kdelibs code. Currently there's no way to figure out what bookmarks were set (or just renamed) by user and what were set by default setup.
Comment 5 Andrey Bondrov 2014-04-21 13:43:18 MSD
New build lists:

https://abf.rosalinux.ru/build_lists/1799843
https://abf.rosalinux.ru/build_lists/1799844

It's still not possible to rename default bookmarks to English names in KDE ("Музыка" -> "Music" etc).

But now SW doesn't translate custom bookmarks.
Comment 6 Dmitry 2014-04-21 14:27:42 MSD
Is is NOT possible to rename default bookmarks (places). That is not a solution.
Comment 7 Dmitry 2014-04-21 14:31:32 MSD
(In reply to comment #4)
> (In reply to comment #3)
> > -1, I would prefer don't apply that patch because:
> > 1) that is really bad code

newItem["caption"] = i18nc("KFile System Bookmarks", bm.text().toUtf8().data());

->

QByteArray chars = bm.text().toUtf8();
newItem["caption"] = i18nc("KFile System Bookmarks", chars.data());
Comment 8 Andrey Bondrov 2014-04-21 15:01:22 MSD
(In reply to comment #7)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > -1, I would prefer don't apply that patch because:
> > > 1) that is really bad code
> 
> newItem["caption"] = i18nc("KFile System Bookmarks",
> bm.text().toUtf8().data());
> 
> ->
> 
> QByteArray chars = bm.text().toUtf8();
> newItem["caption"] = i18nc("KFile System Bookmarks", chars.data());

Doesn't make any sense.
Comment 9 Dmitry 2014-04-21 15:14:58 MSD
It does matter. Your code is undefined behavior.
Comment 10 Andrey Bondrov 2014-04-21 15:44:44 MSD
(In reply to comment #9)
> It does matter. Your code is undefined behavior.

This code comes from kfileplacesitem.cpp (from void KFilePlacesItem::setBookmark(const KBookmark &bookmark) ) and has been tested for years. 

And there's nothing more undefined in 

char *data = bm.text().toUtf8().data();

than in

QString text = bm.text();
QByteArray chars = text.toUtf8();
char *data = chars.data();
Comment 11 Dmitry 2014-04-21 16:15:39 MSD
Tested over the years code does not mean that the code is right. Because many programmers make that mistake. 

char *data = bm.text().toUtf8().data();
do_some(data);

You are dereferencing the wild pointer (it is undefined behavior), because bm.text().toUtf8() creates temporary and the compiler can destroy it when it is unused. "data" can become wild pointer.
Comment 12 Andrey Bondrov 2014-04-21 16:48:21 MSD
(In reply to comment #11)
> Tested over the years code does not mean that the code is right. Because
> many programmers make that mistake. 
> 
> char *data = bm.text().toUtf8().data();
> do_some(data);
> 
> You are dereferencing the wild pointer (it is undefined behavior), because
> bm.text().toUtf8() creates temporary and the compiler can destroy it when it
> is unused. "data" can become wild pointer.

Makes sense for a longer usage. 

But in i18nc("KFile System Bookmarks", bm.text().toUtf8().data()) we use the pointer right after creating it. And use only one.

Even with -Wall and -Wextra there are no warnings about this line.

Anyway, just commit whatever you think should be there. It's your software, not mine... But please note that we need to close this bug ASAP.
Comment 13 Dmitry 2014-04-21 17:08:44 MSD
> Makes sense for a longer usage. 

??? The C++ standard disagrees with you.

> But in i18nc("KFile System Bookmarks", bm.text().toUtf8().data()) we use the
> pointer right after creating it. And use only one.
The data pointed with pointer may be destroyed before i18nc() is executed.

> 
> Even with -Wall and -Wextra there are no warnings about this line.
A compiler can't understand that situation.
 
> Anyway, just commit whatever you think should be there. It's your software,
> not mine... But please note that we need to close this bug ASAP.
Sorry, but it is your patch, not mine. Your patch works because GCC don't destroy temporary ASAP and an application should not translate strings from system.
Comment 14 Andrey Bondrov 2014-04-21 17:25:11 MSD
(In reply to comment #13)
> and an application should not translate strings from system

How do you expect these stings to be translated then?

Of course it's easy to modify kbookmark.cc in this way:

- return text;
+ return ki18nc("KFile System Bookmarks", text.toUtf8()).toString();

But this will break kfileplaces', Dolphin' etc logic.
Comment 15 Dmitry 2014-04-21 17:35:58 MSD
Do you know what code translates bookmarks?
Comment 16 Vladimir Potapov 2014-04-21 17:46:12 MSD
Мужики, выдайте уж какой-нибудь рабочий результат, хватит крутостью мерятся, надо r3 выпускать и чтоб при первом запуске все было переведено.
Остальное вторично.
Comment 17 Postnikov Dmitry 2014-04-21 17:48:36 MSD
Тише! У них тут "сражение на саблях". :)
Comment 18 Andrey Bondrov 2014-04-21 17:59:46 MSD
(In reply to comment #16)
> Мужики, выдайте уж какой-нибудь рабочий результат, хватит крутостью мерятся,
> надо r3 выпускать и чтоб при первом запуске все было переведено.
> Остальное вторично.

http://bugs.rosalinux.ru/show_bug.cgi?id=3985#c5
Comment 19 Andrey Bondrov 2014-04-21 18:06:03 MSD
(In reply to comment #15)
> Do you know what code translates bookmarks?

Yes, various code in various places. It's quite a mess in KDE.

For example, this one from kdelibs/kfile/kfileplacesitem.cpp 

=========================================

void KFilePlacesItem::setBookmark(const KBookmark &bookmark)
{
    m_bookmark = bookmark;
 
    if (bookmark.metaDataItem("isSystemItem") == "true") {
        // This context must stay as it is - the translated system bookmark names
        // are created with 'KFile System Bookmarks' as their context, so this
        // ensures the right string is picked from the catalog.
        // (coles, 13th May 2009)
 
        m_text = i18nc("KFile System Bookmarks", bookmark.text().toUtf8().data());
    } else {
        m_text = bookmark.text();
    }
}

http://quickgit.kde.org/?p=kdelibs.git&a=blob&h=1cb0fa58c82d520e51f1446b699bc1efd99c5a6d&hb=471f91e98c0c1f7cf7c8abcae3ae97cb025f146e&f=kfile%2Fkfileplacesitem.cpp

=========================================

or this one from dolphin/src/panels/places/placesitemmodel.cpp

=========================================

                const KUrl url = bookmark.url();
                if (missingSystemBookmarks.contains(url)) {
                    missingSystemBookmarks.remove(url);
 
                    // Try to retranslate the text of system bookmarks to have translated
                    // items when changing the language. In case if the user has applied a custom
                    // text, the retranslation will fail and the users custom text is still used.
                    // It is important to use "KFile System Bookmarks" as context (see
                    // createSystemBookmarks()).
                    item->setText(i18nc("KFile System Bookmarks", bookmark.text().toUtf8().data()));
                    item->setSystemItem(true);
                }

http://quickgit.kde.org/?p=kde-baseapps.git&a=blob&h=baa770dfdb8d1b2fc79ec313c10fbb5bc38fbd79&hb=d22f3b16ab84710f3af8d49b8efded13d18a8af0&f=dolphin%2Fsrc%2Fpanels%2Fplaces%2Fplacesitemmodel.cpp

=========================================

and so on.
Comment 20 Dmitry 2014-04-21 18:16:48 MSD
(In reply to comment #16)
> Мужики, выдайте уж какой-нибудь рабочий результат, хватит крутостью мерятся,
> надо r3 выпускать и чтоб при первом запуске все было переведено.
> Остальное вторично.

Выше есть хоть какой-то вариант.
Comment 21 Vladimir Potapov 2014-04-21 19:04:25 MSD
rosa-launcher-2.0.0-55.5
http://abf-downloads.rosalinux.ru/rosa2012.1/container/1799843/i586/main/release/
http://abf-downloads.rosalinux.ru/rosa2012.1/container/1799844/x86_64/main/release/
************************** Advisory **************************
Fix bug with SimpleWelcome translates bookmarks named in English
**************************************************************
QA Verified