-
Notifications
You must be signed in to change notification settings - Fork 57
Fix dialog/authentication issues for Edge + 365 and Desktop versions of Office #144
base: master
Are you sure you want to change the base?
Conversation
This will allow for finer control over handling of each browser agent
We will continue to use localStorage as a means of communication between windows
@almgong While you are waiting for Louis to look at this, I have a question about this part of your description: "...since add-ins would wait for message events, while dialogs would only set an entry in localStorage" Are you saying that the messageParent API does not work when Edge is the viewer? |
@Rick-Kirkham For the line that you quoted, I was referring to the the different ways that the parent add-in and the dialog were listening and sending messages to each other, respectively. The library handles these two operations differently depending on the determined host, and it is critical that this resolved value is the same in multiple windows for the dialog to close and communicate data successfully. On the Edge/Office clients that we used to test this fix, we found that the call to |
@almgong , I'm facing the same issue, but I use the CDN to load the library into the code, do you know how can I get your fix as a compiled JS file(office.helpers(.min).js)? |
@alaaet One way would be to clone the forked repository, navigate to the relevant branch |
src/helpers/dialog.ts
Outdated
private _edgeDialog(): Promise<T> { | ||
return new Promise((resolve, reject) => { | ||
Office.context.ui.displayDialogAsync(this.url, { width: this.size.width$, height: this.size.height$ }, (result: Office.AsyncResult) => { | ||
if (result.status === Office.AsyncResultStatus.Failed) { | ||
reject(new DialogError(result.error.message, result.error)); | ||
} | ||
else { | ||
this._pollLocalStorageForToken(resolve, reject); | ||
} | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a bug with this dialog on the Outlook desktop client (tested build 1908 & 1909). I use the fork this PR is based on, and my changes in my fork of the fork(🙄) seems to have fixed it.
The problem seem to be that the dialog isn't properly closed as far as office.js
is concerned. It works as expected when using OWA in Edge, but not on the desktop client.
The scenario is the user is authenticated through the popup dialog right. And at some point the token is invalid and offce-helpers
tries to open a new popup to authenticate the user again. But it receives an error from office.js with code 12007 "A dialog box is already opened from this host window."
These changes seems to have fixed the problem:
private _edgeDialog(): Promise<T> {
return new Promise((resolve, reject) => {
Office.context.ui.displayDialogAsync(this.url, { width: this.size.width$, height: this.size.height$ }, (result: Office.AsyncResult) => {
+ let dialog = result.value as Office.DialogHandler;
+
if (result.status === Office.AsyncResultStatus.Failed) {
reject(new DialogError(result.error.message, result.error));
}
else {
- this._pollLocalStorageForToken(resolve, reject);
+ const onToken = token => {
+ if (dialog) {
+ dialog.close();
+ }
+
+ return resolve(token);
+ };
+
+ this._pollLocalStorageForToken(onToken, reject);
}
});
});
@lindalu-MSFT Seeing as you were the last committer on this repository, could we please get some information as to the future of this repository? |
The handler for Edge environments used to poll for localStorage, but due to issues relating to Edge WebView2 rollout, localStorage is no longer shared between the dialog prompt and add-in pane. This change essentially reverts the original change from 2019 that updated the handling to rely on localStorage. The reason we are doing this is because: 1. The authentication flow is broken until MS resolves this bug: OfficeDev/office-js#1741 2. messageParent has always been the preferred communication medium as noted in the DialogApi documentation: https://docs.microsoft.com/en-us/office/dev/add-ins/develop/dialog-api-in-office-add-ins 3. There was a finding that the dialog needs to be opened with a URL that matches the opener. With this change, messageParent now appears to be working correctly.
Fixes #143
This fix is intended to address authentication issues for both Office 365 with Edge and the desktop versions of Office as of the recent 1903 Windows update, paired with the 1907 update of Office.
We found that for the online version of Office apps running on Edge, the taskpane instance of add-ins would believe that it were running in an add-in environment, but the dialog pop-up would determine that it was in a web environment. This mismatch prevents add-ins from communicating with the dialogs they open for authentication, since add-ins would wait for message events, while dialogs would only set an entry in localStorage.
The fix in this PR is to separate the handling of Edge as a special case and to rely solely on localStorage for both the desktop and online versions of Office apps. We decided to do this because we experienced inconsistent message sending behavior on Edge, whereas localStorage had proven to be more stable.