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

Allow webview guests to be resized manually #7658

Merged
merged 3 commits into from Nov 15, 2016

Conversation

Projects
None yet
4 participants
@poiru
Member

poiru commented Oct 18, 2016

This adds the disableguestresize property for webviews to prevent the
webview guest from reacting to size changes of the webview element. This
also partially documents the webContents.setSize function in order to
manually control the webview guest size.

These two features can be combined to improve resize performance for
e.g. webviews that span the entire window. This greatly reduces the lag
described in #6905.

@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Oct 18, 2016

Member

I'm happy to write tests if this approach looks decent. What do you think, @zcbenz?

To try it out, use e.g. the following and compare with the example in #6905:

index.js:

'use strict';

const {app, BrowserWindow, webContents} = require('electron');
const path = require('path');

app.on('ready', () => {
  const win = new BrowserWindow({
    width: 500,
    height: 500,
    webPreferences: {
      nodeIntegration: true
    },
  });
  win.loadURL('file://' + path.join(__dirname, 'index.html'));

  win.on('resize', () => {
    const [width, height] = win.getContentSize();
    for (let wc of webContents.getAllWebContents()) {
      if (wc.hostWebContents && wc.hostWebContents.id == win.webContents.id) {
        wc.setGuestSize(width, height);
      }
    }
  });
});

index.html:

<!DOCTYPE html>
<html>
<style>
html, body, webview {
  flex: 1;
  margin: 0;
  width: 100%;
  height: 100%;
}

body {
  background-color: red;
}
</style>
<body>
<webview src="https://github.com" disableguestresize></webview>
</body>
</html>
Member

poiru commented Oct 18, 2016

I'm happy to write tests if this approach looks decent. What do you think, @zcbenz?

To try it out, use e.g. the following and compare with the example in #6905:

index.js:

'use strict';

const {app, BrowserWindow, webContents} = require('electron');
const path = require('path');

app.on('ready', () => {
  const win = new BrowserWindow({
    width: 500,
    height: 500,
    webPreferences: {
      nodeIntegration: true
    },
  });
  win.loadURL('file://' + path.join(__dirname, 'index.html'));

  win.on('resize', () => {
    const [width, height] = win.getContentSize();
    for (let wc of webContents.getAllWebContents()) {
      if (wc.hostWebContents && wc.hostWebContents.id == win.webContents.id) {
        wc.setGuestSize(width, height);
      }
    }
  });
});

index.html:

<!DOCTYPE html>
<html>
<style>
html, body, webview {
  flex: 1;
  margin: 0;
  width: 100%;
  height: 100%;
}

body {
  background-color: red;
}
</style>
<body>
<webview src="https://github.com" disableguestresize></webview>
</body>
</html>
@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Oct 24, 2016

Contributor

The whole PR looks good to me, some tests would be awesome. 👍

Contributor

zcbenz commented Oct 24, 2016

The whole PR looks good to me, some tests would be awesome. 👍

@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Oct 25, 2016

Member

@zcbenz Added tests, but seems like they are failing on Linux/Windows. I'll investigate.

Member

poiru commented Oct 25, 2016

@zcbenz Added tests, but seems like they are failing on Linux/Windows. I'll investigate.

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Oct 26, 2016

Contributor

@poiru The tests are failing on CI machines:

not ok 530 <webview> tag disableguestresize attribute resizes guest when attribute is not present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
not ok 531 <webview> tag disableguestresize attribute does not resize guest when attribute is present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
not ok 532 <webview> tag disableguestresize attribute dispatches element resize event even when attribute is present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
Contributor

zcbenz commented Oct 26, 2016

@poiru The tests are failing on CI machines:

not ok 530 <webview> tag disableguestresize attribute resizes guest when attribute is not present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
not ok 531 <webview> tag disableguestresize attribute does not resize guest when attribute is present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
not ok 532 <webview> tag disableguestresize attribute dispatches element resize event even when attribute is present
  Error: timeout of 20000ms exceeded. Ensure the done() callback is being called in this test.
      at C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:232:19
@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Nov 1, 2016

Member

@zcbenz Tests fixed! I also increased the webview spec timeout to 60s since the previous 20s timeout is apparently not enough on Travis.

Member

poiru commented Nov 1, 2016

@zcbenz Tests fixed! I also increased the webview spec timeout to 60s since the previous 20s timeout is apparently not enough on Travis.

@zeke

zeke approved these changes Nov 1, 2016

Docs and tests look good to me!

@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Nov 9, 2016

Member

@kevinsawicki Do you think this can be merged? Would love to have this in the next Electron release.

Member

poiru commented Nov 9, 2016

@kevinsawicki Do you think this can be merged? Would love to have this in the next Electron release.

@kevinsawicki kevinsawicki assigned kevinsawicki and unassigned zcbenz Nov 15, 2016

poiru and others added some commits Oct 25, 2016

Allow webview guests to be resized manually
This adds the `disableguestresize` property for webviews to prevent the
webview guest from reacting to size changes of the webview element. This
also partially documents the `webContents.setSize` function in order to
manually control the webview guest size.

These two features can be combined to improve resize performance for
e.g. webviews that span the entire window. This greatly reduces the lag
described in #6905.

@kevinsawicki kevinsawicki merged commit 0ef6d46 into electron:master Nov 15, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Nov 15, 2016

Contributor

Thanks for this @poiru, it is great to have this supported 👍 🚢

Contributor

kevinsawicki commented Nov 15, 2016

Thanks for this @poiru, it is great to have this supported 👍 🚢

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