OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
690 stars 95 forks source link

[Outlook] Dialog API broken when displaying in iframe in new OWA #599

Closed ShannonLCapper closed 5 years ago

ShannonLCapper commented 5 years ago

In the "new experience" in OWA, the dialog API is not working correctly when dialogs are displayed with displayInIframe: true. The displayDialogAsync API works correctly the first time a dialog is opened with displayInIframe: true, but after that dialog is closed, subsequent calls to displayDialogAsync will fail with status code 5001 (An internal error has occurred.). However, this is especially bad because the dialog will indeed open, just without the taskpane receiving the DialogHandler object to be able to interact with it.

Expected Behavior

I would expect displayDialogAsync to call its provided callback with a success result and a DialogHandler instance when it is called multiple times (as long as the dialog was closed in between each call). I would also expect that the dialog would NOT open if displayDialogAsync returned an error result and no DialogHandler, as this prevents the taskpane from being able to interact with the dialog.

Current Behavior

displayDialogAsync with displayInIframe: true works as expected when it is called the first time. However, after that dialog is closed, subsequent calls to displayDialogAsync with displayInIframe: true return the following result:

Screen Shot 2019-07-03 at 12 33 43 PM

The dialog still opens in this case, and appears to be functional, but it cannot interact with the taskpane since the taskpane never received a DialogHandler instance on which to attach event handlers.

This behavior does not reproduce with dialogs displayed with displayInIframe: false, nor does it reproduce in the "old" experience in OWA.

Steps to Reproduce

  1. Create a plugin that runs in a compose-mode email message, and include an HTML file for a dialog. Add the following script to the task pane's JS file:
    
    renderButton = () => {
    const button = document.createElement(`button`);
    button.textContent = `Open dialog`;
    document.body.appendChild(button);
    button.addEventListener(`click`, openDialog);
    }

openDialog = () => { Office.context.ui.displayDialogAsync(, { width: 80, height: 80, displayInIframe: true, }, (result) => { console.log(result); }); }

Office.initialize = renderButton;

The dialog's HTML file could look something like this:

<!doctype html>

rAF-bug-demo-dialog The dialog rendered!

2. Toggle on `Try the new Outlook` from the OWA inbox
3. Open a new compose mail, and open the task pane.
4. Click the button that says `Open dialog`. Note that the dialog opens fine, and the taskpane console.logs something like this:
<img width="410" alt="Screen Shot 2019-07-03 at 12 45 39 PM" src="https://user-images.githubusercontent.com/21230261/60620602-7f69c380-9d90-11e9-9a63-7028a579e09a.png">
5. Close the dialog with the X button. Notice that OWA throws the following error:
<img width="413" alt="Screen Shot 2019-07-03 at 12 46 32 PM" src="https://user-images.githubusercontent.com/21230261/60620654-9f998280-9d90-11e9-8495-c6d51aa3e143.png">
6. Click the `Open dialog` button again. The dialog will still open, but the taskpane will console.log the following:
<img width="409" alt="Screen Shot 2019-07-03 at 12 47 34 PM" src="https://user-images.githubusercontent.com/21230261/60620698-c35cc880-9d90-11e9-91ad-b8340506ff9b.png">

If you replace step 5 with the taskpane calling `dialogHandlerInstance.close()`, the issue will still reproduce, but OWA will not throw the `Cannot read property 'indexOf' of undefined` error.

Closing and reopening the taskpane will reset the problem (the first call to `displayDialogAsync` will succeed, subsequent calls will fail).

## Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us come up with a solution that is most useful in the real world -->
This is a very severe error for us since our plugin heavily relies on the dialog API. Since `displayDialogAsync` does not return a `DialogHandler` instance with this bug, our code breaks in multiple ways:
1. We're unable to attach `DialogMessageReceived` handlers to the dialog, so messages sent from our dialog get lost into the ether.
2. We think an error occurred while trying to open the dialog and that the dialog didn't open, so our taskpane shows user-facing error UI asking the user to try again.
3. Our taskpane is unable to detect when the dialog is closed by the user, which prevents it from doing certain behaviors it would normally do on dialog close.
3. Our taskpane turns on and off some code paths while a dialog is open. These code paths don't run since we don't know there is an open dialog.

This is a very broken experience for our users, so I really hope you can help us resolve this issue quickly!

## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Platform [PC desktop, Mac, iOS, Office Online]: Office Online
* Host [Excel, Word, PowerPoint, etc.]: Outlook
* Office version number: 16.0.10908.10000
* Operating System: MacOS High Sierra 10.13.6
* Browser (if using Office Online): only tested in Chrome 75.0.3770.100
ShannonLCapper commented 5 years ago

Looks like this may be the same issue seen here https://github.com/OfficeDev/office-js/issues/595

nathguen commented 5 years ago

@ShannonLCapper Thanks for leaving additional info that I failed to add. Between the two issues, I think it tells a pretty complete picture of the problem. 😄

As a work around, I found that this issue was only occurring on the web client. The outlook desktop client appears to be unaffected by the issue (at least from my testing).

kbrandl commented 5 years ago

@ShannonLCapper sorry to hear that you've run into this problem; thanks for providing such detailed information.

@danielgwilson assigning this one to you for investigation (since it's related to Dialog API) -- please reassign if it should go to someone else. Also, please note that this issue is related to #595 (which @nathguen opened yesterday).

exextoc commented 5 years ago

@ShannonLCapper Thanks for reporting this and proving such detailed information. We have added this to our backlog. Unfortunately, we have no timelines to share at this point

ShannonLCapper commented 5 years ago

@kbrandl and @exextoc, has this been backlogged, or is it actively being worked on (as the fix pending label suggests)? This issue provides a really broken experience for our users so I sincerely hope there's something we could do to get it fixed in the near future. Please let us know if there's anything we can do on our end to help!

kbrandl commented 5 years ago

@ShannonLCapper it's in the backlog (not yet actively being worked on, as far as I'm aware); unfortunately we have no timelines to share at this point for precisely when it'll be addressed. Apologies for the confusion re the fix pending label; within this repository, that label is applied to any issue that's "on our list" to be addressed (regardless of whether or not it's actively being worked on yet).

exextoc commented 5 years ago

Thanks for reporting this issue. we have checked-in the fix for this, will be live soon.

kbrandl commented 5 years ago

@exextoc thanks for the update! @ShannonLCapper clearly I was out of the loop re the status of this particular issue; my apologies for the earlier miscommunication!

ShannonLCapper commented 5 years ago

@kbrandl, that's absolutely no problem. I'm just glad that you all were able to come up with a fix so quickly!

Bartlebyy commented 5 years ago

@exextoc @kbrandl any update on when the next release will be? We're also affected by this bug.

david-davidson commented 5 years ago

@exextoc @kbrandl @lliu113 Hi there! It looks like this fix is live; I can no longer reproduce the problem. 🎉

Is it correct that the fix has shipped?

kbrandl commented 5 years ago

@david-davidson thanks for reaching out about this.

@exextoc -- can you please confirm whether (or not) the fix for this issue has shipped? If it has shipped, please comment and close this issue. Thanks!

exextoc commented 5 years ago

Yes, we shipped the fix. Thanks @david-davidson for confirming that problem is resolved for you.