D14513: Add QML Extensions API

[David Rosca]

drosca added inline comments.

INLINE COMMENTS

> qmlkeyevent.cpp:25
> +{
> +    QQmlEngine::setObjectOwnership(this, QQmlEngine::JavaScriptOwnership);
> +}

You already call `newQObject` on all events,  which makes them JavaScriptOwnership, so it isn't needed here.

> qmlextensionscheme.cpp:31
> +        QmlWebEngineUrlRequestJob *request = new QmlWebEngineUrlRequestJob(job);
> +        emit requestStarted(qmlEngine(this)->newQObject(request));
> +    });

You can still use the QmlWebEngineUrlRequestJob pointer in signal, not QJSValue.

> qmlfileutils.cpp:37
> +    }
> +    const QUrl resolvedUrl = QUrl::fromLocalFile(path).resolved(QUrl::fromEncoded(filePath.toUtf8()));
> +    const QString resolvedPath = resolvedUrl.toLocalFile();

Why does it use QUrl?

You can just do path + / + filePath and then QDir::cleanPath.

> anmolgautam wrote in qmli18n.cpp:26
> > If locale is an empty string, "", each part of the locale that should be modified is  set according  to  the environment variables.

And is that really needed?
I'd like to see explicitly choosing which language to load here.

REPOSITORY
  R875 Falkon

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

To: anmolgautam, drosca
Cc: falkon, herrold, anmolgautam, SGOrava, iodelay, spoorun, ptabis, navarromorales, cochise, clivej, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/falkon/attachments/20180811/cb011aad/attachment.html>