Bug 895103

Summary: Provide native dialog for showDialog() UI plugin API instead of browser window
Product: Red Hat Enterprise Virtualization Manager Reporter: Chris Morrissey <cmorriss>
Component: RFEsAssignee: Vojtech Szocs <vszocs>
Status: CLOSED ERRATA QA Contact: Jiri Belka <jbelka>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 3.2.0CC: acathrow, dyasny, ecohen, eedri, iheim, jbiddle, jdonohue, jrankin, lpeer, sgrinber, vszocs
Target Milestone: ---Keywords: Improvement
Target Release: 3.2.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: ux
Fixed In Version: sf14 Doc Type: Enhancement
Doc Text:
Previously, the user interface plugin, showDialog API, was implemented using browser-specific (non-modal) pop up windows; that is, windows opened via standard window.open JavaScript API. As a result, custom dialogs triggered by user interface plugins did not look and feel the same as standard WebAdmin dialogs, were non-modal, and could be closed simply via browser-specific popup window close icon. Now, the showDialog API is implemented using WebAdmin-specific dialog infrastructure, allowing custom dialogs triggered by user interface plugins to look and feel the same as standard WebAdmin dialogs, be modal just like standard WebAdmin dialogs, and provide control over closing the custom dialog. Additionally, there is now an API for updating the dialog content URL, as well as closing the custom dialog programatically (via JavaScript). This change also adds support for cross-window cross-origin communication that can be utilized by user interface plugins, such as custom content. It also talks back to the plugin code via a WebAdmin (parent) window. This enables custom content to pass arbitrary messages back to given plugin(s).
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-06-10 21:43:28 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Sample UI plugin that demonstrates cross-window communication feature none

Description Chris Morrissey 2013-01-14 15:09:47 UTC
Description of problem:
Currently when the showDialog() function for UI plugins is used, it launches supplied URL in a separate browser window. This makes the plugin look and act differently then other parts of the admin UI. The URL should be shown in a normal oVirt dialog with the content of the URL in an iframe.

Version-Release number of selected component (if applicable):
3.2.0

How reproducible:
Launch a URL in a dialog from a UI plugin using the showDialog() function.

Steps to Reproduce:
1. Create a basic test UI plugin.
2. In the UI plugin HTML, add a tab action that launches a dialog similar to the following:

UiInit: function() {
    api.addMainTabActionButton('Storage', 'Provision NetApp Domain', {
         onClick: function() {
              api.showDialog('Provision NetApp Storage Domain', 'http://redhat.com', 600, 600);
         }
    });
}

3. In the web admin UI, click the new tab action in the storage.
  
Actual results:
The redhat web page will load in a browser window.

Expected results:
It should be loaded in a native oVirt/RHEVM dialog.

Additional info:
The NetApp team working on using this functionality can also test it to ensure it is functioning properly is several different scenarios.

Comment 1 Chris Morrissey 2013-01-14 15:25:11 UTC
Because the content of the dialog is in an iframe whose content is potentially be served from a different domain, it should provide a messaging API using the HTML 5 Window.postMessage() functionality. This can be used to notify to the dialog that it should close.

OK and Cancel buttons could be optionally added to the dialog and used to close it, although for our purposes this is not necessary. We will always have our own OK and Cancel buttons so we can better control whether OK is allowed based on the validation of the content in the dialog.

Comment 3 Vojtech Szocs 2013-02-05 17:08:26 UTC
First (work-in-progress) version of the patch is available at http://gerrit.ovirt.org/#/c/11717/

There are still some TODO items that need to be addressed, these are outlined in the commit message of the patch.

As for HTML5 Window.postMessage communication scheme, here's the current proposal:
* iframe (attached to top-level DOM) does postMessage <pluginName>:<apiExpression>, e.g. foo-plugin:closeDialog('my-dialog')
* WebAdmin receives the message, obtains pluginApi for <pluginName> and invokes <apiExpression> on it, e.g. fooPluginApi.closeDialog('my-dialog')

The above scheme allows *any* iframe (showDialog, addMainTab, etc.) showing custom content to invoke arbitrary plugin API function (expression). This means that custom content can interact directly with plugin API on its own.

Chris, can you please test the first version of the patch, and let me know what you think about HTML5 Window.postMessage design proposal?

Comment 7 Chris Morrissey 2013-02-28 14:12:11 UTC
I have downloaded and test the first version of the patch. It works great and I've been able to use HTML5 window.postMessage() using my own plugin, so it definitely does work. I think your design for a standard communication scheme would definitely work. It's also relatively easy to implement the code receiving the message and ultimately calling the closeDialog() in my start.html page. So if that doesn't make it into this release it's not a problem.

Comment 8 Vojtech Szocs 2013-03-08 13:23:16 UTC
Hi Chris, I apologize for getting back to this issue so late..

> It's also relatively easy to implement the code receiving the message and ultimately calling the closeDialog() in my start.html page

If I understand correctly, in your start.html page, you register postMessage listener function [addEventListener/attachEvent] in top-level WebAdmin window, e.g. 'parent'?

If so, I'd rather recommend not doing so, as it can interfere with standard postMessage communication scheme we're planning to implement as part of this issue (your plugin would replace existing standard postMessage listener function).

> So if that doesn't make it into this release it's not a problem.

Actually, I think it's better to have proposed postMessage communication scheme implemented into target release, so that you (or anyone else) don't have to deal with processing postMessage events on your own.

Aside from "<pluginName>:<apiExpression>" message format, used to invoke plugin API directly from custom content, we could also support the use-case when custom content fires postMessage and plugin code (start.html) can react to it. For example, you click OK button in your dialog, and besides closing the dialog via plugin API, you could also pass some data to plugin (start.html) - what do you think?

Comment 9 Vojtech Szocs 2013-03-08 17:28:39 UTC
I've just realized that we can make the postMessage communication scheme completely transparent from plugin's point of view, which benefits both WebAdmin and UI plugins.

Instead of WebAdmin trying to process "<pluginName>:<expression>" messages on its own, it can just relay "<expression>" to the given plugin via new event handler function, for example:

MessageReceived: function(postMessageEvent) {
    // Might also check postMessageEvent.origin to guard against
    // malicious content that could do postMessage calls for this plugin
    if (postMessageEvent.data == 'closeMyDialog') {
        api.closeDialog('my-dialog');
    }
}

I really like this idea, as it makes the communication completely transparent, and it's the UI plugin which does the actual API call. With this approach, it's also possible to guard against malicious content, which could try to impersonate "real" content by doing postMessage calls for given plugin.

Comment 10 Chris Morrissey 2013-03-08 18:26:22 UTC
I agree with having a MessageReceived function as a standard that plugins could use to receive messages. Also, I would recommend requiring the plugins to register any domain that they allow messages from and have the base framework check the origin for them. This alleviates the need to have every plugin check as well as ensures that a plugin cannot receive a message if they haven't set the origins from which they can receive messages.

So, this would include adding something like the following to the API:

api.registerMessageDomain('http://nicedomain.com:8080');

Comment 11 Vojtech Szocs 2013-03-13 18:04:52 UTC
I agree with everything Chris wrote in comment #10.

Allowed message origin registration could be generalized into a function that provides extra options to plugin-specific API instance.

For example:

// Plugins MUST specify 'messageOrigins' API option in order to
// receive messages via MessageReceived event handler function
api.options({
    // Can use '*' to accept messages from any origin, as per specification
    // Using '*' is not recommended for relaying confidential information
    messageOrigins: ['http://nicedomain.com:8080/', 'http://anotherdomain.com/']
});

api.register({
    // 'message' argument is taken directly from the message event,
    // it can be string or anything else that was sent via postMessage
    // Note: some browsers support string messages only [1]
    MessageReceived: function(message) {
        // We already passed origin check here, just process the message
        if (message == 'closeMyDialog') {
            api.closeDialog('my-dialog');
        }
    }
});

[1] http://stackoverflow.com/questions/13761968/detect-whether-postmessage-can-send-objects

Comment 12 Vojtech Szocs 2013-03-21 14:35:19 UTC
Good news: patch [http://gerrit.ovirt.org/#/c/11717/] is now finalized and should be tested before merging. Chris, can you please take this patch and give it a spin?

HTML5 Window.postMessage communication scheme will be implemented in a separate patch, according to details per comment #11.

Comment 15 Vojtech Szocs 2013-04-10 16:39:01 UTC
The second patch [http://gerrit.ovirt.org/#/c/13794/] is now finalized and tested on my local environment. Chris, can you please assist with testing this patch?

Comment 16 Vojtech Szocs 2013-04-15 16:52:23 UTC
Created attachment 735987 [details]
Sample UI plugin that demonstrates cross-window communication feature

Comment 17 Jiri Belka 2013-05-06 12:40:28 UTC
ok, sf15 (tested with vszocs@ standing behind my back).

Comment 20 errata-xmlrpc 2013-06-10 21:43:28 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-0888.html