<table><tr><td style="">broulik updated this revision to Diff 37139.<br />broulik edited the summary of this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-iqkxa4m3ga62x2v/">(Show Details)</a><br />broulik edited the test plan for this revision. <a href="https://phabricator.kde.org/transactions/detail/PHID-XACT-DREV-ylmsbc546df2lra/">(Show Details)</a><br />broulik added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D13878">View Revision</a></tr></table><br /><div><div><ul class="remarkup-list">
<li class="remarkup-list-item">Remove child right after adding it, fixes YouTube and other video player breakage, also avoids leaking those DOM elements in the <tt style="background: #ebebeb; font-size: 13px;">head</tt> element</li>
</ul></div></div><br /><div><strong>CHANGES TO REVISION SUMMARY</strong><div><div style="white-space: pre-wrap; color: #74777D;">Spotify web player uses a `video` tag instead of `audio` to play its music.<br />
<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">We can do this as `<head>` is invisible by default and if a page wants to show the player in the page, it has to manually add it to the DOM anyway,</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">Also remove the element after adding it.</span> <span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">but we add it before we even return it from `createElement` so the order is correct here</span><span style="padding: 0 2px; color: #333333; background: rgba(151, 234, 151, .6);">This ensures the element returned to the caller is not part of the DOM but it is sufficient for our `MutationObserver` to see it and keep a reference to it in order to control it</span>.<div style="padding: 8px 0;">...</div></div></div></div><br /><div><strong>CHANGES TO TEST PLAN</strong><div><div style="white-space: pre-wrap; color: #74777D;"><span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);">Verified that you can `appendChild` the same element to different elements and it just moves around instead of e.g. an "already attached to a parent" error<br />
</span>I can control Spotify Web player now.<span style="padding: 0 2px; color: #333333; background: rgba(251, 175, 175, .7);"><br />
When pressing play from the media controller, the title changes to the album name, as Spotify updates the page title only after the player started playing and we don't monitor the title tag yet, but this is unrelated to this patch<br />
Didn't notice anything unusual while casually browsing the web</span></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R856 Plasma Browser Integration</div></div></div><br /><div><strong>CHANGES SINCE LAST UPDATE</strong><div><a href="https://phabricator.kde.org/D13878?vs=37133&id=37139">https://phabricator.kde.org/D13878?vs=37133&id=37139</a></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D13878">https://phabricator.kde.org/D13878</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>extension/content-script.js</div></div></div><br /><div><strong>To: </strong>broulik, Plasma, davidedmundson, fvogt<br /><strong>Cc: </strong>plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>