OneDrive / onedrive-api-docs

Official documentation for the OneDrive API
MIT License
451 stars 228 forks source link

OneDrive JS FilePicker in Electron error 'Cannot create property 'href' on string #863

Closed nab911 closed 1 year ago

nab911 commented 6 years ago

First off, the JS FilePicker is not listed in this repository. Any way it can get added?

When importing the OneDrive.js filepicker and following the guide, everything works as expected (except one JS error which i'll describe later). But, when opening in our Electron app, the window loads and closes throwing a js error, Cannot create property 'href' on string. This happens because electron's popup window sets window.location = 'onedriveUrl'. Then the script tries to update window.location.href and crashes out. I was able to fix this by dropping .href on the call in the minified script but would like to actually PR my changes.

There was also another issue of e.data.indexOf is not a function. That was fixed via adding an additional e.data.indexOf null check.

Would also like to get an unminified version I could fully test and propose changes to.

etyp commented 6 years ago

@ificator this one kills us on redirects for other platforms. Wondering if this is something that is sensible to move on?

ghost commented 6 years ago

I'm facing the same issue with onedrive file picker in electron.

TL;DR; OneDrive filepicker JS is incompatible with Electron.

The problem is on this line -

e.getCurrentPopup().getPopupWindow().location.href = s;

In a browser instance e.getCurrentPopup().getPopupWindow().location returns a URL object but in electron it returns a string.

I tried fixing this to e.getCurrentPopup().getPopupWindow().location = s but then it breaks on another error.

The next error I get is related to the use RequireJS. RequireJS tries to replace NodeJS' require but it cannot do so under electron. This fails with its face on the floor. I have spent an entire day monkey patching this script but it just leads to more and more errors.

ghost commented 6 years ago

@aditima hey there! Is there any update on this one?

KevinTCoughlin commented 6 years ago

Hey @harshilsharma63 was busy getting #891 fix out the door. Looking into this 🕐 . Thank you for the helpful debug as a kickstart 👍 .

etyp commented 6 years ago

Awesome news, thanks for addressing this one @KevinTCoughlin

We're happy to help however possible (testing, debugging, etc) just let me know.

KevinTCoughlin commented 6 years ago

@etyp if you have a sample repo that shows as far as you can get before the above errors are thrown by the lib that would save me some time. If not no worries, trying to do that myself now.

ghost commented 6 years ago

@KevinTCoughlin this might be helpful - https://github.com/harshilsharma63/hosted/blob/master/onedrive_fixed.js

It contains the fixes I've been talking about.

KevinTCoughlin commented 6 years ago

@harshilsharma63 are you using an http/s endpoint for your redirectUri? If so how? Electron serves from file:// breaking the OAuth flow for me, unless you're using a local web server as a proxy for auth.

ghost commented 6 years ago

@KevinTCoughlin I think you'd need to open a popup window for authenticating? That's what we did for another project for the oAuth flow.

Anrock commented 5 years ago

Here is issue on Electon-side about location.href: https://github.com/electron/electron/issues/15017

KevinTCoughlin commented 5 years ago

Currently, the OneDrive File Picker's supported browser matrix does not include Electron.

The OneDrive picker and saver supports the following web browsers:

  • Internet Explorer Desktop & Mobile 11+
  • Microsoft Edge v25+
  • Chrome Desktop 5+
  • Chrome for Android
  • Android Browser 4.1+
  • Firefox Desktop & Mobile 8+ Safari Desktop & Mobile 5+

https://docs.microsoft.com/en-us/onedrive/developer/controls/file-pickers/js-v72/?view=odsp-graph-online#supported-browsers

We are aware of this limitation and understand the need for compatibility within non-traditional browser contexts. It is an enhancement we are working towards.

nab911 commented 5 years ago

@KevinTCoughlin any way we can get this added to this repo unminified? I would be willing to open a PR with the fixes but we need the original source.

chetanyakan commented 4 years ago

@nab911 If you are still looking for a fix to this, you can try to make changes to the more readable debug version of JS FilePicker: http://js.live.net/v7.2/OneDrive.debug.js

Interestingly, Slack is able to run the file picker in their electron app with the OneDrive-for-Slack app. They seem to be using a modified version of the JS file picker. But their version does not work in other electron apps and requires some other changes I was not able to figure out.

The React Component (NPM Package) does seem to work with Electron. But it only has a preview release yet and the License is not as open as the JS FilePicker.

patrick-rodgers commented 1 year ago

As part of a repository clean up effort we are closing older issues. If this issue remains, please: open a new issue, reference this issue, and provide any additional details that may help in resolution. Thank you for your understanding as we work to improve our responsiveness.