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

use double quote around ${url} #15282

Merged
merged 1 commit into from
Jan 2, 2019
Merged

use double quote around ${url} #15282

merged 1 commit into from
Jan 2, 2019

Conversation

songgao
Copy link
Contributor

@songgao songgao commented Jan 2, 2019

No description provided.

@songgao songgao requested a review from a team January 2, 2019 18:59
@@ -43,12 +43,14 @@ video {
}
`

// Double quote around ${url} is necessary as encodeURIComponent encodes double
// quote but not single quote.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not why though, right? "${url}" is equivalent to url, whereas '${url}' just sets that string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I don’t understand the question. These are inside the injected JavaScript, so they are both strings. ${url} is populated through the back-tick quote around the injected JavaScript.

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, to clarify:

v.setAttribute('src', "${url}")

is equivalent to:

v.setAttribute('src', url)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I think I follow what you're saying. I missed the backticks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jzila v.setAttribute('src', url) would be equivalent to v.setAttribute('src', http://127.0.0.1:xxxx/blahblah). This is not evaluated by node directly, but inside the webview.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually it's not equivelant to that. It'd just be v.setAttribute('src', url) and webview would not be able to find url

@songgao songgao merged commit 9b78102 into master Jan 2, 2019
@songgao songgao deleted the songgao/quick-fix-20190102 branch January 2, 2019 21:27
mmaxim pushed a commit that referenced this pull request Jan 2, 2019
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.

3 participants