Opened 8 years ago

Closed 5 years ago

#8995 closed New feature (fixed)

Add support for https to sitemaps

Reported by: John Hensley Owned by: Aymeric Augustin
Component: contrib.sitemaps Version: master
Severity: Normal Keywords: sitemaps
Cc: miracle2k, John Hensley, mmitar@…, Marcel Eyer Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I needed to put HTTPS URLs in my sitemaps, and wanted to ping search engines other than Google. The attached patch adds those enhancements.

Attachments (5)

sitemaps.diff (7.3 KB) - added by John Hensley 6 years ago.
Patch against r13357 for sitemap enhancements.
search-commit-msg (2 bytes) - added by John Hensley 6 years ago.
Ignore this.
https_sitemaps.diff (13.0 KB) - added by John Hensley 5 years ago.
Patch to support HTTPS URLs in sitemaps
patch_1_sitemaps_https_documentation.diff (1.4 KB) - added by Marco Paolini 5 years ago.
documentation
django-sitemap-protocol.diff (15.1 KB) - added by John Hensley 5 years ago.
Backward-compatible patch with docs

Download all attachments as: .zip

Change History (31)

comment:1 Changed 8 years ago by Adrian Holovaty

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Sounds like a reasonable enhancement. It'll have to wait until we have a 1.0.x branch. I have not examined this patch.

comment:2 Changed 7 years ago by miracle2k

Cc: miracle2k added

comment:3 Changed 7 years ago by miracle2k

It seems yahoo is pinged twice by this patch, once with the sitemap url (super call), and then again with the base url. Any particular reason for this?

comment:4 in reply to:  3 Changed 6 years ago by John Hensley

Replying to miracle2k:

It seems yahoo is pinged twice by this patch, once with the sitemap url (super call), and then again with the base url. Any particular reason for this?

Well, I'd like to think it was because in trying to get Yahoo! to crawl my sites I found it necessary, but it's been so long I don't remember, and now it does seem redundant and spammy. The overridden ping method can probably be removed entirely, so just the sitemap is submitted.

comment:5 Changed 6 years ago by John Hensley

Cc: John Hensley added

I updated the patch to work with recent code. For now I just removed Yahoo, as the ping service seems to be broken and is always returning 403 Forbidden. It's not possible that I reached their rate limit, I tried from multiple IPs, and it seems to be reported pretty widely. Some claim you now need to get an app ID and use the update notification service instead. If that's true, Yahoo hasn't updated their docs.

At any rate, I'm afraid that's all the time I have for this at the moment, so Yahoo's out.

Changed 6 years ago by John Hensley

Attachment: sitemaps.diff added

Patch against r13357 for sitemap enhancements.

Changed 6 years ago by John Hensley

Attachment: search-commit-msg added

Ignore this.

comment:6 Changed 6 years ago by Mitar

Cc: mmitar@… added

comment:7 Changed 6 years ago by Marcel Eyer

Cc: Marcel Eyer added

comment:8 Changed 6 years ago by Gabriel Hurley

Component: Contrib appscontrib.sitemaps

comment:9 Changed 5 years ago by Julien Phalip

Easy pickings: unset
Needs tests: set
Severity: Normal
Type: New feature

comment:10 Changed 5 years ago by Jannis Leidel

Patch needs improvement: set
UI/UX: unset

Those are really two features in one ticket, please decide which you want to cover here and open a new ticket for the other feature. Thanks.

comment:11 Changed 5 years ago by John Hensley

Resolution: wontfix
Status: newclosed

Sorry, don't have time to spend on this one now. After three years, it's clearly not needed; let's just get it off the list.

comment:12 Changed 5 years ago by Jannis Leidel

I think the protocol attribute is definitely useful, IMO.

comment:13 Changed 5 years ago by Mitar

+1

comment:14 Changed 5 years ago by Aymeric Augustin

Resolution: wontfix
Status: closedreopened

Let's decide this ticket is only about HTTPS URLs in sitemaps (something I've missed in the past...) and forget about Yahoo, which no longer offers a "ping" feature.

comment:15 Changed 5 years ago by Jannis Leidel

Summary: django.contrib.sitemaps enhancementsAdd support for https to sitemaps

Changing the title..

Changed 5 years ago by John Hensley

Attachment: https_sitemaps.diff added

Patch to support HTTPS URLs in sitemaps

comment:16 Changed 5 years ago by John Hensley

All right, that's easier. This latest patch just adds HTTPS support.

comment:17 Changed 5 years ago by vsemenov

Thanks for the patch! This will fix "URLs not followed" warning in Google Webmaster Tools for HTTPS locations in sitemaps.

comment:18 Changed 5 years ago by Jannis Leidel

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:19 Changed 5 years ago by Chris Beaven

One question, the current patch changes the index view to use the sitemap's protocol for the site map URL locations rather than just relying on request.is_secure(). Isn't this slightly backwards incompatible?

comment:20 Changed 5 years ago by Jannis Leidel

That's backwards incompatible indeed, but the feature request is valid. The sitemap class should check if a protocol is specified as a class attribute and fallback to the request.is_secure check. In doubt this could all be a method called get_protocol or similar to allow the users to customize things.

comment:21 Changed 5 years ago by Jannis Leidel

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Changed 5 years ago by Marco Paolini

documentation

comment:22 Changed 5 years ago by Marco Paolini

added some documentation for this new feature

Changed 5 years ago by John Hensley

Backward-compatible patch with docs

comment:23 Changed 5 years ago by John Hensley

Patch needs improvement: unset

I've added a patch that includes the docs (thanks, Marco) and falls back to the request protocol if the sitemap doesn't specify it.

comment:24 Changed 5 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin
Status: reopenednew

comment:25 Changed 5 years ago by Aymeric Augustin

There are several major issues with the current patch:

  • it isn't acceptable to slap a request attribute on the Sitemap object and then magically use it within Sitemap.get_urls
  • duplicating all the tests isn't a sustainable approach
  • it seems to me that the patch doesn't actually work because the request attribute is only added in views.index, not views.sitemap

I'm currently working on a more invasive patch that should eventually result in cleaner code. I'll upload a patch when it is ready.

comment:26 Changed 5 years ago by Aymeric Augustin

Resolution: fixed
Status: newclosed

In [17409]:

Fixed #8995 -- Added support for HTTPS in sitemaps.

Modularized tests and did a bit of cleanup while I was in the area.

Note: See TracTickets for help on using tickets.
Back to Top