dfaure added a comment.

  Looks good, just some details.

INLINE COMMENTS

> plugin_babelfish.cpp:225
> +          part->url().scheme().compare(QLatin1String("https"), 
> Qt::CaseInsensitive) == 0 ) &&
> +        (part->inherits("KHTMLPart") || part->inherits("KWebKitPart"))) {
>          m_menu->setEnabled(true);

This is missing WebEngine btw, but instead this should be generalized with 
something like
"the part has a browser extension", like this:

  && KParts::BrowserExtension::childObject(part)

> plugin_babelfish.cpp:273
> +    if (url.scheme() == "https") {
> +        if (KMessageBox::warningContinueCancel(0L,
> +                                               xi18nc("@info", "\

the 0L should be part->widget() instead.

REPOSITORY
  R226 Konqueror

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

To: marten, #konqueror, #plasma, dfaure
Cc: plasma-devel, #dolphin, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart, lukas

Reply via email to