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

Add 'webpreferences' attribute to webview #7631

Merged
merged 7 commits into from Oct 25, 2016

Conversation

Projects
None yet
4 participants
@pfrazee
Contributor

pfrazee commented Oct 14, 2016

Per #7431 and #2749, this PR adds a webpreferences attribute to the webview tag. The string is implemented using the same parsing logic as the features string in window.open.

@@ -184,6 +184,32 @@ const attachGuest = function (embedder, elementInstanceId, guestInstanceId, para
disableBlinkFeatures: params.disableblinkfeatures
}
// parse the 'webpreferences' attribute string, if set
// this uses the same parsing rules as window.open uses for its features

This comment has been minimized.

@kevinsawicki

kevinsawicki Oct 24, 2016

Contributor

Might be nice to use a common helper method for this so the parsing stays consistent between the two APIs.

@kevinsawicki

kevinsawicki Oct 24, 2016

Contributor

Might be nice to use a common helper method for this so the parsing stays consistent between the two APIs.

This comment has been minimized.

@pfrazee

pfrazee Oct 24, 2016

Contributor

I factored out the function into the /lib/common dir, but I can't seem to make the require('../common/parse-features-string') happen in /lib/browser/guest-view-manager.js. I'm guessing this is due to changes to the require search paths. Any suggestions?

@pfrazee

pfrazee Oct 24, 2016

Contributor

I factored out the function into the /lib/common dir, but I can't seem to make the require('../common/parse-features-string') happen in /lib/browser/guest-view-manager.js. I'm guessing this is due to changes to the require search paths. Any suggestions?

This comment has been minimized.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

Any suggestions?

After looking into this a bit more, the code for each wouldn't be the exact same since window.open handles additional features as well which the webpreferences would not, so it might not make sense to have a common function between the two.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

Any suggestions?

After looking into this a bit more, the code for each wouldn't be the exact same since window.open handles additional features as well which the webpreferences would not, so it might not make sense to have a common function between the two.

This comment has been minimized.

@pfrazee

pfrazee Oct 25, 2016

Contributor

I had the same thought, but I did find a way to make it work, that I think is pretty clean. I just need to solve the require() situation.

@pfrazee

pfrazee Oct 25, 2016

Contributor

I had the same thought, but I did find a way to make it work, that I think is pretty clean. I just need to solve the require() situation.

This comment has been minimized.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

@pfrazee want to push it up and I'll take a look at the require issue?

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

@pfrazee want to push it up and I'll take a look at the require issue?

This comment has been minimized.

@pfrazee

pfrazee Oct 25, 2016

Contributor

Sure

@pfrazee

pfrazee Oct 25, 2016

Contributor

Sure

This comment has been minimized.

@pfrazee

pfrazee Oct 25, 2016

Contributor

Pushed

@pfrazee

pfrazee Oct 25, 2016

Contributor

Pushed

This comment has been minimized.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

@pfrazee The issue was that new JS files need to be added to filenames.gypi to be included in the built version.

I pushed a couple changes to this pull request, please let me know what you think.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

@pfrazee The issue was that new JS files need to be added to filenames.gypi to be included in the built version.

I pushed a couple changes to this pull request, please let me know what you think.

This comment has been minimized.

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

but I did find a way to make it work, that I think is pretty clean

Yeah, I really like the way you did it, it makes both use cases easy to follow 👍

@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

but I did find a way to make it work, that I think is pretty clean

Yeah, I really like the way you did it, it makes both use cases easy to follow 👍

This comment has been minimized.

@pfrazee

pfrazee Oct 25, 2016

Contributor

Yeah looks much better.

@pfrazee

pfrazee Oct 25, 2016

Contributor

Yeah looks much better.

@kevinsawicki kevinsawicki self-assigned this Oct 24, 2016

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

Thanks @pfrazee for implementing this 👍 🚢 :shipit:

Contributor

kevinsawicki commented Oct 25, 2016

Thanks @pfrazee for implementing this 👍 🚢 :shipit:

@kevinsawicki kevinsawicki merged commit 39a5c7d into electron:master Oct 25, 2016

0 of 2 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
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Oct 25, 2016

Contributor

@pfrazee beaker looks really cool, interested in adding it to http://electron.atom.io/apps/ ?

Contributor

kevinsawicki commented Oct 25, 2016

@pfrazee beaker looks really cool, interested in adding it to http://electron.atom.io/apps/ ?

@pfrazee

This comment has been minimized.

Show comment
Hide comment
@pfrazee

pfrazee Oct 25, 2016

Contributor

Thanks @pfrazee for implementing this 👍 🚢 :shipit:

@kevinsawicki you bet!

@pfrazee beaker looks really cool, interested in adding it to http://electron.atom.io/apps/ ?

Yeah that'd be great!

Contributor

pfrazee commented Oct 25, 2016

Thanks @pfrazee for implementing this 👍 🚢 :shipit:

@kevinsawicki you bet!

@pfrazee beaker looks really cool, interested in adding it to http://electron.atom.io/apps/ ?

Yeah that'd be great!

@ea2973929

This comment has been minimized.

Show comment
Hide comment
@ea2973929

ea2973929 Jan 18, 2017

Hi, this seemed to be exactly what I would need to change the default font of a webview using webpreferences. However, the setting is defaultFontFamily takes an object as argument (https://github.com/electron/electron/blob/master/docs/api/browser-window.md). How can I encode that as a parameter for the webpreferences attribute?

ea2973929 commented Jan 18, 2017

Hi, this seemed to be exactly what I would need to change the default font of a webview using webpreferences. However, the setting is defaultFontFamily takes an object as argument (https://github.com/electron/electron/blob/master/docs/api/browser-window.md). How can I encode that as a parameter for the webpreferences attribute?

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
upgrade: `electron` to v1.6.6
Some changes that we're particularly interested in:

- electron/electron#8590
- electron/electron#8852
- electron/electron#7631

Note that the `electron-prebuilt` packaged has been renamed to
`electron`.

Change-Type: patch
Changelog-Entry: Upgrade Electron to v1.6.6.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
upgrade: `electron` to v1.6.6 (#1357)
Some changes that we're particularly interested in:

- electron/electron#8590
- electron/electron#8852
- electron/electron#7631

Note that the `electron-prebuilt` packaged has been renamed to
`electron`.

Change-Type: patch
Changelog-Entry: Upgrade Electron to v1.6.6.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
@lihang1870719

This comment has been minimized.

Show comment
Hide comment
@lihang1870719

lihang1870719 Nov 26, 2017

I also account the same things like ea29738929。Is anyone figure out?

lihang1870719 commented Nov 26, 2017

I also account the same things like ea29738929。Is anyone figure out?

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