New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposes more Handoff related APIs to Electron #9869

Merged
merged 8 commits into from Sep 14, 2017

Conversation

Projects
None yet
6 participants
@rafaelnobrepd
Contributor

rafaelnobrepd commented Jun 26, 2017

Hi guys, thanks for the amazing project!
I'm taking a stab at improving Handoff support as in #9622
My C++ and Js skills are close to zero, so please bear with me!
I attempted mimicking the change points of a few PRs and added what we needed: support for invalidating the current activity, updating the current activity user info and a few delegate callbacks as events.
It looks to work when running the app with command electron ./electron-source

    const electron = require('electron');
    const { app } = electron;

    [...]

    console.log(app.invalidateCurrentActivity);
    //output: function invalidateCurrentActivity() { [native code] }

However, Handoff will only work when the app is properly signed with the Team ID, and in this case my implementation doesn't seem to work:
Running App after package with electron-packager

    const electron = require('electron');
    const { app } = electron;

    [...]

    dialog.showMessageBox(mainWindow, {
      message: app.invalidateCurrentActivity.toString()
    });
    /**
    A JavaScript error occurred in the main process
    Uncaught Exception:
    TypeError: Cannot read property 'toString' of undefined
        at EventEmitter.<anonymous> (/Users/Shared/myApp/releases/mas-preview/MyApp-mas-x64/My App.app/Contents/Resources/app.asar/js/main.min.js:1:385981)
        at emitTwo (events.js:106:13)
        at EventEmitter.emit (events.js:191:7)
        at WebContents.<anonymous> (/Users/Shared/myApp/releases/mas-preview/My App-mas-x64/My App.app/Contents/Resources/electron.asar/browser/api/web-contents.js:247:37)
        at emitTwo (events.js:106:13)
        at WebContents.emit (events.js:191:7)
    */

It would be awesome to get some feedback on what may be missing here to work. I downloaded libchromium-static and ran bootstrap with the -u option as it would never finish downloading within the script itself.
I took the out/R/Electron.app and replaced the one in the dist folder when packaging.

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jul 14, 2017

TypeError: Cannot read property 'toString' of undefined

It looks like this method is just missing and so you perhaps it wasn't packaged correctly when copied from your local build.

What would be the test way to test this locally when reviewing? Do you have a small sample app that shows how you plan on using these new events/APIs?

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Jul 27, 2017

Hey! Thanks for helping.
We figured the packaging problem (we weren't aware of the ~/.electron source for electron versions)
We are now having problems with sandboxing with the latest version. I found from the sources that what defines a mas electron build is the MAS_BUILD env variable, we're now downloading the mas flavored libchromiumcontent to test a new build with this (I originally just built it but it presented those sandboxing errors, then when cleaning and bootstrapping again I realized it needed another libchromium to build with MAS support).
We'll get back with a modified quick-starter demo project demonstrating Handoff usage after we can get it to work for further feedback!

* `webpageURL` String (optional) - The webpage to load in a browser if no suitable app is
installed on the resuming device. The scheme must be `http` or `https`.
Invalidates de current Handoff user activity.

This comment has been minimized.

@zeke

zeke Jul 27, 2017

Member

typo: de

This comment has been minimized.

@rafaelnobrepd

rafaelnobrepd Jul 27, 2017

Contributor

Fixed, thanks

* `type` String - Uniquely identifies the activity. Maps to
[`NSUserActivity.activityType`][activity-type].
* `userInfo` Object - App-specific state to store for use by another device.

This comment has been minimized.

@zeke

zeke Jul 27, 2017

Member

The description for the webpageURL argument is missing. Also I think Url is generally preferred over URL in the Electron codebase, e.g. webpageUrl

This comment has been minimized.

@rafaelnobrepd

rafaelnobrepd Jul 27, 2017

Contributor

Oh, this actually came in by mistake after copying documentation from setUserActivity. This method does not have any parameters.

@zeke

This comment has been minimized.

Member

zeke commented Jul 27, 2017

For the uninitiated:

Handoff is a capability introduced in iOS 8 and OS X v10.10 that transfers user activities among multiple devices associated with the same user. In iOS 9 and OS X v10.11, Handoff helps your app participate in search by making it possible to designate user activities and app states as searchable. For example, when a searchable activity or state appears in Spotlight search results or Siri suggestions, users can tap the result to return to the relevant area in your app.

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Jul 31, 2017

Here is the forked quick-starter with added Handoff API usage (for now only the invalidateCurrentActivity)

We hit a roadblock trying to create a working electron packaged app with our build. We added the $MAS_BUILD env variable, cleaned and bootstrapped electron to get it to include the latest mas libchromiumcontent, built to release, zipped the output and updated the SHASUM file on ~/.electron, then we have electron-packager fail with the following:

Packaging app for platform mas x64 using electron v1.7.5
fs.js:652
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
                 ^

Error: ENOENT: no such file or directory, open '/var/folders/bc/dp6n5v917zgc5mbykbtv_hwc0000gn/T/electron-packager/mas-x64/Dummy App Handoff-mas-x64/Electron.app/Contents/Info.plist'
    at Object.fs.openSync (fs.js:652:18)
    at Object.fs.readFileSync (fs.js:553:33)
    at MacApp.loadPlist (/Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/mac.js:83:27)
    at MacApp.updatePlistFiles (/Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/mac.js:95:26)
    at buildMacApp (/Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/mac.js:269:18)
    at /Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/common.js:311:7
    at end (/Users/nobre/Documents/electron-quick-start/node_modules/run-series/index.js:8:15)
    at done (/Users/nobre/Documents/electron-quick-start/node_modules/run-series/index.js:11:10)
    at each (/Users/nobre/Documents/electron-quick-start/node_modules/run-series/index.js:16:43)
    at /Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/common.js:86:7
    at FSReqWrap.CB [as oncomplete] (/Users/nobre/Documents/electron-quick-start/node_modules/electron-packager/node_modules/fs-extra/lib/remove/rimraf.js:57:5)

Any pointers as to what may have gone wrong?

@zeke

This comment has been minimized.

Member

zeke commented Aug 1, 2017

cc @malept who may have seen this before with electron-packager.

@malept

This comment has been minimized.

Member

malept commented Aug 1, 2017

That's new. Does the MAS zip have the Contents/Info.plist file in the right place?

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Aug 1, 2017

Yes, it has. When I put back the original mas-x64 zip file, it packages correctly tho.
I'm using the output of a Release build, only exchanging the Electron.app bundle and generating a new zip file, using the $MAS_BUILD and latest corresponding libchromiumcontent mas/x64/7a9d4a1c9c265468dd54005f6c1920b2cc2c8ec3, I suspected maybe an issue with the current master or libchromium version?

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Aug 1, 2017

@rafaelnobrepd Can you upload your generated zip file?

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Aug 1, 2017

@rafaelnobrepd That zip has been generated badly, the zip file should contain the app file at the top level of the zip. Your generated zip contains a single folder which then contains the .app file.

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Aug 1, 2017

Oh, didn't notice that. OSX unzips the original into a folder with the same name, that was my bad. Will try with the structure you mentioned.

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Aug 1, 2017

It works! 👍
We'll proceed to test all the added functionality, thanks

@rafaelnobrepd

This comment has been minimized.

Contributor

rafaelnobrepd commented Aug 9, 2017

Alright, we have debugged and fixed the main API points we have to propose.
I updated the docs with the expected usage pattern, and updated our demo repo with a working sample and instructions.

Please let us know if our approach is in line with Electron.

@rafaelnobrepd rafaelnobrepd changed the title from [WIP] Exposes more Handoff related APIs to Electron to Exposes more Handoff related APIs to Electron Aug 9, 2017

@zcbenz zcbenz merged commit 0784090 into electron:master Sep 14, 2017

6 of 8 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-mas-x64 Build #5096 in progress...
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
electron-osx-x64 Build #5079 succeeded in 12 min
Details
@welcome

This comment has been minimized.

welcome bot commented Sep 14, 2017

Congrats on merging your first pull request! 🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment