Skip to content
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 support for css in content_scripts #10076

Merged

Conversation

alexstrat
Copy link
Contributor

No description provided.

Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, can you rebase the commit on master?

@jkleinsc jkleinsc self-requested a review August 7, 2017 17:45
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexstrat can you rebase this to resolve the conflict?
Also, could you provide tests for this?

@zcbenz zcbenz force-pushed the add/add-support-for-content_scripts.css branch from 3b0d710 to e1a232e Compare September 14, 2017 08:24
@zcbenz
Copy link
Member

zcbenz commented Sep 14, 2017

I have rebased the commit and am merging the pull request, currently we don't have a way to test extension APIs, and adding tests for it is better to be put in another pull request.

@zcbenz zcbenz closed this Sep 14, 2017
@zcbenz zcbenz reopened this Sep 14, 2017
@zcbenz zcbenz merged commit fc99785 into electron:master Sep 14, 2017
for (const {code} of script.css) {
process.once('document-end', () => {
var node = document.createElement('style')
node.innerHTML = code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to use node.textContent = code instead to ensure child elements aren't created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good suggestion, do you want to create a PR for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants