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

Our # behavior in routing is invalid per spec #2445

Closed
dead-claudia opened this issue Jun 27, 2019 · 0 comments · Fixed by #2448
Closed

Our # behavior in routing is invalid per spec #2445

dead-claudia opened this issue Jun 27, 2019 · 0 comments · Fixed by #2448
Assignees
Labels
Type: Bug For bugs and any other unexpected breakage
Milestone

Comments

@dead-claudia
Copy link
Member

dead-claudia commented Jun 27, 2019

Mithril Version: v1.0.0 to v2.0.0-rc.6

Expected Behavior

Mithril to only tolerate valid URLs provided the prefix is valid

Current Behavior

Mithril can interpret #!/foo/bar?one=1#two=2 as a valid URL when it isn't.

Possible Solution

We should stop interpreting that # as part of the "parameters", to align with spec and to push users to use only valid URLs. 😉

What we should be doing is aligning with how it's handled at the network level and just stripping it if it's present.

Steps to Reproduce

N/A - this is a design bug

Context

Discovered per here. Apparently, outside the Mithril world, people have already run into issues relying on the fact browsers tolerate multiple #s in URLs, but are apparently inconsistent in how they handle them at times.

Also, this means we can save a few bytes in the pathname parsing.

Additional Information

Your Environment

  • Browser Name and version:
  • Operating System and version (desktop or mobile):
  • Link to your project:
@dead-claudia dead-claudia added Type: Bug For bugs and any other unexpected breakage m.request labels Jun 27, 2019
@dead-claudia dead-claudia added this to the 2.0.0 milestone Jun 27, 2019
@dead-claudia dead-claudia self-assigned this Jun 27, 2019
dead-claudia added a commit to dead-claudia/mithril.js that referenced this issue Jul 3, 2019
- Valid URLs must not contain a `#` within its fragment.
  MithrilJS#2445
- Our docs were a little confusing and misleading - `m.pathname` isn't
  aware of URLs, just path names.
- Removed the relevant extension to `m.parsePathname` required to
  support the hash parsing extension. Now we just shave it off and
  ignore it.
- Fix support for arbitrary prefixes, so prefixes like `?#` are
  handled correctly.
- Add a bunch of tests to cover various areas of confusion and unusual
  edge cases.
dead-claudia added a commit to dead-claudia/mithril.js that referenced this issue Jul 3, 2019
- Valid URLs must not contain a `#` within its fragment.
  MithrilJS#2445
- Our docs were a little confusing and misleading - `m.pathname` isn't
  aware of URLs, just path names.
- Removed the relevant extension to `m.parsePathname` required to
  support the hash parsing extension. Now we just shave it off and
  ignore it.
- Fix support for arbitrary prefixes, so prefixes like `?#` are
  handled correctly.
- Add a bunch of tests to cover various areas of confusion and unusual
  edge cases.
dead-claudia added a commit to dead-claudia/mithril.js that referenced this issue Jul 3, 2019
- Valid URLs must not contain a `#` within its fragment.
  MithrilJS#2445
- Our docs were a little confusing and misleading - `m.pathname` isn't
  aware of URLs, just path names.
- Removed the relevant extension to `m.parseQueryString` required to
  support the hash parsing extension. Now we just shave it off and
  ignore it.
- Fix support for arbitrary prefixes, so prefixes like `?#` are
  handled correctly.
- Add a bunch of tests to cover various areas of confusion and unusual
  edge cases.
dead-claudia added a commit that referenced this issue Jul 3, 2019
* Clarify pathname docs, follow spec with fragments

- Valid URLs must not contain a `#` within its fragment.
  #2445
- Our docs were a little confusing and misleading - `m.pathname` isn't
  aware of URLs, just path names.
- Removed the relevant extension to `m.parseQueryString` required to
  support the hash parsing extension. Now we just shave it off and
  ignore it.
- Fix support for arbitrary prefixes, so prefixes like `?#` are
  handled correctly.
- Add a bunch of tests to cover various areas of confusion and unusual
  edge cases.

* Update with PR [skip ci]
@dead-claudia dead-claudia moved this to Closed in Triage/bugs Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug For bugs and any other unexpected breakage
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

1 participant