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

query string in worker URL #1175

Closed
1 of 3 tasks
domdir opened this issue May 2, 2020 · 1 comment · Fixed by #1208
Closed
1 of 3 tasks

query string in worker URL #1175

domdir opened this issue May 2, 2020 · 1 comment · Fixed by #1208
Labels

Comments

@domdir
Copy link
Contributor

domdir commented May 2, 2020

Problem
When trying to create a worker agent on a document location that has a non empty query string, the query string is included in the resource URL of the worker. This leads to a resource URL that cannot be found.

Steps To Reproduce

  1. Go to a page that spawns a worker
  2. Add some query string and reload the page
  3. Worker resource URL cannot be found

Expected behavior
Query string is not included in the worker's resource URL.

Screenshots
Error in console with query string ?homescreen:
grafik

Environment:

  • Yew version [master]

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later

The issue is in

let href = utils::document().location().unwrap().href().unwrap();

The href should be replaced with either:

  • page origin
    or
  • page origin + pathname

While page origin + pathname is closer to the way it behaves currently, I think the correct solution, however, is to only use the page origin. This way the resource URL is independent of the current path. But I am happy to implement either one: maintainer's choice!

@domdir domdir added the bug label May 2, 2020
@jstarry
Copy link
Member

jstarry commented May 11, 2020

Sorry for the delay, I agree that origin is the way to go. Thank you @domdir!

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

Successfully merging a pull request may close this issue.

2 participants