Opened 8 years ago
Last modified 11 days ago
#29030 assigned Cleanup/optimization
Make construction of "View on site" and "View Site" URLs consistent
| Reported by: | Kaleb Hornsby | Owned by: | Pierre Sassoulas |
|---|---|---|---|
| Component: | contrib.admin | Version: | 2.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The How Django Uses the Sites Framework documentation states the following:
In the admin framework, the “view on site” link uses the current Site to work out the domain for the site that it will redirect to.
This statement seems to be false. In the admin site docs, it states the following:
The URL for the “View site” link at the top of each admin page. By default,
site_urlis/. Set it toNoneto remove the link.
For sites running on a subpath, theeach_context()method checks if the current request hasrequest.META['SCRIPT_NAME']set and uses that value ifsite_urlisn’t set to something other than/.
This seems to be backed up by what I am actually seeing, and what the code says that it is doing. Changing the current site's domain in the sites framework has no effect on this link. I suggest removing the incorrect information from the sites documentation. The alternative would be to add this functionality to the admin site.
Change History (7)
comment:2 by , 8 years ago
So when the previous docs change was made, the only references to django.contrib.sites with the admin app were in views/template.py and views/doc.py. Neither of them were used to use the appropriate domain in the 'view site' or 'view on site' functionality.
comment:3 by , 8 years ago
And now looking into it further, "view on site" does in deed have this functionality... "view site" does not. This is an inconsistency in my opinion. I should probably close this, and re-open another ticket. I would like to have some input from anybody else whether it the behavior is inconsistent and should be addressed.
follow-up: 5 comment:4 by , 8 years ago
| Summary: | django.contrib.sites Documnets incorrectly its use in django.contrib.admin "View Site" functionality → contrib.sites incorrectly documents its use in django.contrib.admin "View Site" functionality |
|---|
Is your suggestion to change the implementation of a81af7f49de7ff3f51f111de28ed3a682f67ea89 to use the sites framework instead of hardcoding '/'? I'm not exactly sure what case that would fix. If you're browsing a particular site like "www.example.com/admin/" wouldn't the site URL be "www.example.com", same as linking to "/"?
comment:5 by , 8 years ago
Replying to Tim Graham:
Is your suggestion to change the implementation of a81af7f49de7ff3f51f111de28ed3a682f67ea89 to use the sites framework instead of hardcoding '/'? I'm not exactly sure what case that would fix. If you're browsing a particular site like "www.example.com/admin/" wouldn't the site URL be "www.example.com", same as linking to "/"?
Right now if you are hosting at www.example.com that is accessible to the internet, and you have a admin.example.com that is not publicly accessible and in your settings, you have SITE_ID = 1 and sites.models.Site.objects.get_current().domain == 'www.example.com' then when you are on the admin (at https://admin.example.com/admin) and you click on the "View on site" link for an object that defines its get_absolute_url method to return '/things/1/', then you get redirected to https://www.example.com/things/1. If you click on the "View site" link, however, you get sent to https://admin.example.com/.
Now I am not even sure if changing it to check if sites is installed and prepending the domain is desired behavior or not. Anybody can set admin.site.site_url = sites.models.Site.objects.get_current().domain. But when I was reading the documentation as linked in the ticket, I did get confused. It is highly likely that I am the only one to have confused "View on site" with "View site". But maybe not. I think that the two links should probably be consistent with each other by default, though.
Thank you for taking the time to look at this ticket and inquiring more about the issue.
comment:7 by , 8 years ago
| Component: | Documentation → contrib.admin |
|---|---|
| Easy pickings: | unset |
| Summary: | contrib.sites incorrectly documents its use in django.contrib.admin "View Site" functionality → Make construction of "View on site" and "View Site" URLs consistent |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
It might be feasible to make the links consistent, although backwards compatibility could be an issue.
comment:8 by , 11 days ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
Looks like this was the original change: https://github.com/django/django/commit/4fd2b0ca77c135e39b52bd8081067b9365f9fb9f