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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reload/destroy webview guestinstance issues #7852

Merged
merged 6 commits into from Nov 7, 2016

Conversation

Projects
None yet
2 participants
@kevinsawicki
Contributor

kevinsawicki commented Nov 2, 2016

This pull request seeks to address the issues found in #7700 regarding the guestinstance attribute disappearing when hiding/showing webviews. This attribute was added in #7157.

  • 馃悰 Only destroy guest when moving between views. Currently hiding and then showing a webview destroys it web contents because it isn't checking that the guest is actually moving across views.
  • 馃悰 Only load URL and set webview size on first attach event. #7700 switched the listener from a .once to a .on and so currently webviews are reloaded each time they are attached.
  • 馃悰 Guard against the attachedCallback firing while handling a detachedCallback event. This appears to happen when setting the guestinstance attribute and running replaceChild to move a webview node, uncovered by the golden layout example from #7700.
  • Merge #7863 first and rebase this pull request on top of it

Fixes #7700

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Nov 3, 2016

Contributor

@TimNZ would you be able to try out this branch and see if it works for your app? Using your example I couldn't get the guestinstance attribute to clear out anymore on this branch and the behavior now seems to match how pre-1.4.1 behaved.

Contributor

kevinsawicki commented Nov 3, 2016

@TimNZ would you be able to try out this branch and see if it works for your app? Using your example I couldn't get the guestinstance attribute to clear out anymore on this branch and the behavior now seems to match how pre-1.4.1 behaved.

@TimNZ

This comment has been minimized.

Show comment
Hide comment
@TimNZ

TimNZ Nov 3, 2016

@kevinsawicki Sure - please save me hours of pain and tell me how as I ain't no git expert.

TimNZ commented Nov 3, 2016

@kevinsawicki Sure - please save me hours of pain and tell me how as I ain't no git expert.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Nov 3, 2016

Contributor

please save me hours of pain and tell me how as I ain't no git expert.

This should generate a build:

git clone https://github.com/electron/electron
cd electron
git checkout webview-guestinstance
npm run bootstrap -- --dev # will take some time
npm run build # will take some time
npm start -- ./path/to/your/app

Then the app should be opened.

Contributor

kevinsawicki commented Nov 3, 2016

please save me hours of pain and tell me how as I ain't no git expert.

This should generate a build:

git clone https://github.com/electron/electron
cd electron
git checkout webview-guestinstance
npm run bootstrap -- --dev # will take some time
npm run build # will take some time
npm start -- ./path/to/your/app

Then the app should be opened.

@TimNZ

This comment has been minimized.

Show comment
Hide comment
@TimNZ

TimNZ Nov 3, 2016

Looks good! Thanks so much for the quick turn-around.

TimNZ commented Nov 3, 2016

Looks good! Thanks so much for the quick turn-around.

@kevinsawicki kevinsawicki changed the title from [WIP] Fix reload/destroy webview guestinstance issues to Fix reload/destroy webview guestinstance issues Nov 7, 2016

@kevinsawicki kevinsawicki merged commit b6ece7d into master Nov 7, 2016

3 of 7 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-osx-x64 Build #2779 in progress...
Details
electron-win-ia32 Build #1844 in progress...
Details
electron-linux-arm Build #4633947 succeeded in 66s
Details
electron-linux-ia32 Build #4633948 succeeded in 60s
Details
electron-linux-x64 Build #4633949 succeeded in 123s
Details

@kevinsawicki kevinsawicki deleted the webview-guestinstance branch Nov 7, 2016

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