[webkit-reviews] review granted: [Bug 188607] EWS bubbles are being hidden due to lack of space. : [Attachment 347559] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 188607: EWS bubbles are being hidden due to lack of space.
https://bugs.webkit.org/show_bug.cgi?id=188607

Attachment 347559: Patch

https://bugs.webkit.org/attachment.cgi?id=347559&action=review




--- Comment #26 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 347559
  --> https://bugs.webkit.org/attachment.cgi?id=347559
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347559&action=review

> Websites/bugs.webkit.org/template/en/default/attachment/edit.html.tmpl:50
> +<script>
> +window.addEventListener('message', function (e) {
> +    if (e.origin !== 'https://webkit-queues.webkit.org' || !e.data.height)
> +	   return;
> +
> +    const iframe = document.querySelector('.statusBubble > iframe');
> +    iframe.style.height = e.data.height + 'px';
> +    iframe.style.width = e.data.width + 'px';
> +}, false);
> +
> +function handleStatusBubbleLoad(iframe) {
> +    iframe.contentWindow.postMessage('containerMetrics',
'https://webkit-queues.webkit.org');
> +}
> +</script>

This code effectively duplicates similar code in code-review.js. I suggest that
we extract the common code into a shared JavaScript function and put it in its
own JavaScript file that we include from this page and code-review.js. Maybe
something like:

function updateFromStatusBubbleFrame(messageEvent)
 {
    if (messageEvent.origin !== 'https://webkit-queues.webkit.org')
	return;
    if (!messageEvent.data || !messageEvent.data.height)
	return;

    var mayBeFrames =
Array.prototype.slice.call(document.getElementsByClassName('statusBubble'));
    if (!mayBeFrames)
	return;

    const iframeTagName = 'IFRAME';
    function filterFrames(element, index, array) {
	if (element.tagName === iframeTagName)
	    return true;
	if (element.firstElementChild && element.firstElementChild.tagName ===
iframeTagName)
	    return true;
	return false;
    }

    var frameToUpdate = null;
    for (const frame of mayBeFrames.filter(filterFrames)) {
	if (frame.contentWindow === messageEvent.source) {
	    frameToUpdate = frame;
	    break;
	}
    }
    if (!frameToUpdate)
	return;

    frameToUpdate.style.height = e.data.height + 'px';
    frameToUpdate.style.width = e.data.width + 'px';
 }

Then in both this file and code-review.js we would register this function as
the event handler for the DOM MessageEvent:

window.addEventListener('message', updateFromStatusBubbleFrame, false);

> Websites/bugs.webkit.org/template/en/default/attachment/edit.html.tmpl:277
>	   <div class="statusBubble">
>	     <iframe src="https://webkit-queues.webkit.org/status-bubble/[%
attachment.id %]"

If we can move the 'class="statusBubble"' from the <div> to the <iframe> then
we can simplify the code in updateFromStatusBubbleFrame() above.

> Websites/bugs.webkit.org/template/en/default/attachment/edit.html.tmpl:278
> +		     style="width: 600px; height: 20px; border: none;"
scrolling="no" onload="handleStatusBubbleLoad(this);">

This is OK as is. The semicolon (;) is not necessary.

> Websites/bugs.webkit.org/template/en/default/attachment/list.html.tmpl:53
> +window.addEventListener('message', function (e) {
> +    if (e.origin !== 'https://webkit-queues.webkit.org' || !e.data.height)
> +	   return;
> +
> +    for (const iframe of document.querySelectorAll('.statusBubble >
iframe')) {
> +	   if (iframe.contentWindow !== e.source)
> +	       continue;
> +
> +	   iframe.style.height = e.data.height + 'px';
> +	   iframe.style.width = e.data.width + 'px';
> +    }
> +}, false);

This code is similar to code in code-review.js. See my suggestion above for how
we can share this code.