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.

Sign up for GitHub

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

Jump to bottom

Changing locale does not preserve the hash in the url #2962

Closed
jfly opened this issue Jun 14, 2018 · 0 comments · Fixed by #2964
Closed

Changing locale does not preserve the hash in the url #2962

jfly opened this issue Jun 14, 2018 · 0 comments · Fixed by #2964
Assignees
@jfly

Comments

@jfly
Copy link
Contributor

jfly commented Jun 14, 2018

I noticed this while investigating #2959.

The issue is that the HTTP referer header does not include the "hash" part of a url, so this redirect:

worldcubeassociation.org/WcaOnRails/app/controllers/application_controller.rb

Line 46 in 792220e

redirect_to request.referer || root_path
effectively strips the hash part of the url.

I think we actually already handle the change locale request with javascript, so you could imagine that we pass the current url using javascript instead of relying upon the http referer header. I'll experiment with that a bit.
@jfly jfly self-assigned this Jun 14, 2018
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Jun 14, 2018
@jfly
Preserve the url hash when changing locales.
adc90d7 
To implement this, I had to write a custom javascript handler to handle
switching locales. Previously, this was handled for us via Rails's
unobtrusive javascript, but I could not find any way to add in
additional url parameters to the PATCH request the unobtrusive
javascript was generating for us. There's actually an open PR from 2013
to implement support for this here: rails/jquery-ujs#307.

I also rewrote the set_locale_spec to enable javascript and be more like
what I believe a feature spec should be. I actually don't understand how
the spec was even passing before, I would expect it to fail because it
was sending a GET request, and we don't have a handler for GET request
to update the locale. Maybe rspec/capybara/poltergiest has baked in
support for unobtrusive javascript so that the right thing will happen
even if you don't have javascript enabled?? That sounds crazy, but it's
the only thing I can think of. Regardless of how this was working
before, I do believe that the new specs are better.

This fixes thewca#2962.
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Jun 14, 2018
@jfly
Preserve the url hash when changing locales.
e116f80 
To implement this, I had to write a custom javascript handler to handle
switching locales. Previously, this was handled for us via Rails's
unobtrusive javascript, but I could not find any way to add in
additional url parameters to the PATCH request the unobtrusive
javascript was generating for us. There's actually an open PR from 2013
to implement support for this here: rails/jquery-ujs#307.

I also rewrote the set_locale_spec to enable javascript and be more like
what I believe a feature spec should be. I actually don't understand how
the spec was even passing before, I would expect it to fail because it
was sending a GET request, and we don't have a handler for GET request
to update the locale. Maybe rspec/capybara/poltergiest has baked in
support for unobtrusive javascript so that the right thing will happen
even if you don't have javascript enabled?? That sounds crazy, but it's
the only thing I can think of. Regardless of how this was working
before, I do believe that the new specs are better.

This fixes thewca#2962.
@jfly jfly mentioned this issue Jun 14, 2018
Merged
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Jun 14, 2018
@jfly
Preserve the url hash when changing locales.
afd76ad 
To implement this, I had to write a custom javascript handler to handle
switching locales. Previously, this was handled for us via Rails's
unobtrusive javascript, but I could not find any way to add in
additional url parameters to the PATCH request the unobtrusive
javascript was generating for us. There's actually an open PR from 2013
to implement support for this here: rails/jquery-ujs#307.

I also rewrote the set_locale_spec to enable javascript and be more like
what I believe a feature spec should be. I actually don't understand how
the spec was even passing before, I would expect it to fail because it
was sending a GET request, and we don't have a handler for GET request
to update the locale. Maybe rspec/capybara/poltergiest has baked in
support for unobtrusive javascript so that the right thing will happen
even if you don't have javascript enabled?? That sounds crazy, but it's
the only thing I can think of. Regardless of how this was working
before, I do believe that the new specs are better.

This fixes thewca#2962.
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Jun 15, 2018
@jfly
Preserve the url hash when changing locales.
4d913ad 
To implement this, I had to write a custom javascript handler to handle
switching locales. Previously, this was handled for us via Rails's
unobtrusive javascript, but I could not find any way to add in
additional url parameters to the PATCH request the unobtrusive
javascript was generating for us. There's actually an open PR from 2013
to implement support for this here: rails/jquery-ujs#307.

I also rewrote the set_locale_spec to enable javascript and be more like
what I believe a feature spec should be. I actually don't understand how
the spec was even passing before, I would expect it to fail because it
was sending a GET request, and we don't have a handler for GET request
to update the locale. Maybe rspec/capybara/poltergiest has baked in
support for unobtrusive javascript so that the right thing will happen
even if you don't have javascript enabled?? That sounds crazy, but it's
the only thing I can think of. Regardless of how this was working
before, I do believe that the new specs are better.

This fixes thewca#2962.
jfly added a commit that referenced this issue Jun 15, 2018
@jfly
Preserve the url hash when changing locales.
9eed7e7 
To implement this, I had to write a custom javascript handler to handle
switching locales. Previously, this was handled for us via Rails's
unobtrusive javascript, but I could not find any way to add in
additional url parameters to the PATCH request the unobtrusive
javascript was generating for us. There's actually an open PR from 2013
to implement support for this here: rails/jquery-ujs#307.

I also rewrote the set_locale_spec to enable javascript and be more like
what I believe a feature spec should be. I actually don't understand how
the spec was even passing before, I would expect it to fail because it
was sending a GET request, and we don't have a handler for GET request
to update the locale. Maybe rspec/capybara/poltergiest has baked in
support for unobtrusive javascript so that the right thing will happen
even if you don't have javascript enabled?? That sounds crazy, but it's
the only thing I can think of. Regardless of how this was working
before, I do believe that the new specs are better.

This fixes #2962.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jfly jfly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Preserve the url hash when changing locales. jfly/worldcubeassociation.org
1 participant
@jfly