D6717: Expose base64-encoded favicons to the tabsrunner

[Kai Uwe Broulik]

broulik added a comment.


  Cool!

INLINE COMMENTS

> extension.js:495
> +        var sendTabsIfComplete = function() {
> +            if (--total > 0)
> +                return;

Braces even for single line statements:

  if (...) {
      ...
  }

> extension.js:505
> +
> +        for (let tabIndex in tabs) {
> +            let currentIndex = tabIndex; // Not shared

Does this need a `tabs.hasOwnProperty(...)` check? (cf for in being horrible in JS)

> tabsrunner.cpp:188
> +
> +            QString favIconData = tab.value(QStringLiteral("favIconData")).toString();
> +            int b64start = favIconData.indexOf(',');

const

> tabsrunner.cpp:191
> +            if (b64start != -1) {
> +                QByteArray b64 = favIconData.rightRef(favIconData.size() - b64start - 1).toLatin1();
> +                QByteArray data = QByteArray::fromBase64(b64);

+1 for usage of `ref` :)

> tabsrunner.cpp:224
>  
> -            match.setIconName(iconName);
> +            if (!iconName.isEmpty())
> +                match.setIconName(iconName);

Braces

REPOSITORY
  R856 Plasma Browser Integration

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

To: fvogt, #plasma, broulik, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170727/8cb27617/attachment-0001.html>