D23151: Implement Web Share API through Purpose

[Fabian Vogt]

fvogt requested changes to this revision.
fvogt added a comment.
This revision now requires changes to proceed.


  AFAICT this won't work on wayland and will also break the browser's native implementation once that actually exists.

INLINE COMMENTS

> content-script.js:22
> +// from https://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript
> +function guid() {
> +    return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {

`generateGuid`, otherwise it looks like a getter

> content-script.js:837
> +
> +    // navigator.share must only be defined in secure (https) context
> +    if (!window.isSecureContext) {

Where is that documented?

> purposeplugin.cpp:43
> +{
> +    delete m_menu;
> +    m_menu = nullptr;

Just call `onUnload()`?

> purposeplugin.cpp:47
> +
> +bool PurposePlugin::onLoad()
> +{

Can remove this, parent class does that already.

> purposeplugin.cpp:86
> +                    sendPendingReply(false, {
> +                        {QStringLiteral("error"), QStringLiteral("CANCELED")}
> +                    });

Now we have `error`, `errorCode` and `errorMessage`?

> purposeplugin.cpp:131
> +
> +        if (!text.isEmpty()) { // share text only
> +            showShareMenu(shareJson, QStringLiteral("text/plain"));

Merge this with the same if above and put the `KIO::mimetype` query into the other if. Currently it would break if both `url` and `text` are empty.

> purposeplugin.cpp:144
> +
> +    return {};
> +}

No response. IMO this should be handled in the plugin manager though, as discussed on that diff.

> purposeplugin.cpp:153
> +    sendReply(m_pendingReplySerial, reply);
> +    m_pendingReplySerial = 0;
> +}

0 is a valid serial...

> purposeplugin.h:57
> +
> +    Purpose::Menu *m_menu = nullptr;
> +    QNetworkAccessManager *m_manager = nullptr;

`std::unique_ptr`?

> purposeplugin.h:58
> +    Purpose::Menu *m_menu = nullptr;
> +    QNetworkAccessManager *m_manager = nullptr;
> +    QPointer<QNetworkReply> m_mimeReply;

Unused?

> purposeplugin.h:59
> +    QNetworkAccessManager *m_manager = nullptr;
> +    QPointer<QNetworkReply> m_mimeReply;
> +

Unused?

REPOSITORY
  R856 Plasma Browser Integration

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

To: broulik, #plasma, fvogt, davidedmundson, nicolasfella, apol
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190815/f9b0a06d/attachment-0001.html>