D29321: Port and enable the User Agent Changer plugin

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> uachangerplugin.cpp:39
> +
> +#include <kparts/part.h>
> +#include <kparts/openurlarguments.h>

(not needed)

> uachangerplugin.cpp:98
> +    // and plasma-workspace/kcms/translations/translations.cpp
> +    const KConfigGroup grp = KSharedConfig::openConfig("plasma-localerc")->group("Translations");
> +    QStringList languageList = grp.readEntry("LANGUAGE").split(QLatin1Char(':'),

Are you sure the KConfigGroup keeps the KSharedConfig alive? It would be good if it did, but I'm not sure it does.
Putting the KSharedConfig into a local var seems safer to me.

Any reason not to use QLocale().uiLanguages()?

> uachangerplugin.cpp:256
> -        connect(m_applyEntireSiteAction, SIGNAL(triggered()), this, SLOT(slotApplyToDomain()));
> -        m_pUAMenu->menu()->addAction(i18n("Apply to Entire Site"));
>  

(wow that was broken)

> uachangerplugin.cpp:394
> +
> +    QUrl u(QString("http://%1/").arg(hostname));
> +    const QString tld = u.topLevelDomain(QUrl::EncodeUnicode);

could be const too

> uachangerplugin.cpp:396
> +    const QString tld = u.topLevelDomain(QUrl::EncodeUnicode);
> +    if (tld.isEmpty()) return (hostname);			// name has no valid TLD
> +

Please use `return hostname;` without parenthesis. `return` isn't a function.

> uachangerplugin.cpp:401
> +    const QString prev = prefix.mid(idx+1);			// works even if no '.'
> +    return (prev+tld);
>  }

same

REPOSITORY
  R226 Konqueror

REVISION DETAIL
  https://phabricator.kde.org/D29321

To: marten, #konqueror, #plasma, dfaure
Cc: dfaure
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200501/333168cf/attachment.htm>