Opened on 06/21/2017 at 03:18:40 PM

Closed on 08/04/2017 at 06:20:32 PM

#5343 closed change (fixed)

Create global get_canonical_url function in CMS

Reported by: juliandoucette Assignee:
Priority: P2 Milestone:
Module: Sitescripts Keywords:
Cc: wspee, kvas, jsonesen, ire, saroyanm Blocked By:
Blocking: #5444, #5445, #5446, #5447 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29495555/

Description (last modified by kvas)

Background

See codereview.

I would like to be able to output the canonical url (protocol://host.domain:port/path etc) of a page on a page and/or template.

What to change

Create a global function get_canonical_url(page) that accepts a page path and returns a canonical link (that contains no locale component). It should verify that the page exists in default locale and should raise an exception if it doesn't.

Implementation notes:

  • The base url should be configurable in the website settings via siteurl variable that already exists.
  • Most of the code that generates the links already exists in cms/sources.py:Source.resolve_link.

Attachments (0)

Change History (14)

comment:1 Changed on 06/21/2017 at 04:32:00 PM by kvas

  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Ready set

comment:2 Changed on 07/13/2017 at 08:33:10 AM by ire

  • Cc ire added; iaderinokun removed

comment:3 Changed on 07/22/2017 at 12:45:28 PM by juliandoucette

Note:

This is how I created a canonical url on adblockbrowser.org

link rel="canonical" href="https://adblockbrowser.org/{{ page[:-5] if page.split('/')[-1] == 'index' else page }}" />

I did it this way because:

  1. I didn't want a URL ending in "index"
  2. I didn't want the language to be part of the canonical URL

I think 1 should be default behaviour and 2 should be configurable. e.g.

get_page_url(page, withLocale)

comment:4 Changed on 07/22/2017 at 07:52:10 PM by juliandoucette

  • Blocking 5447 added

comment:5 Changed on 07/22/2017 at 07:54:21 PM by juliandoucette

  • Blocking 5446 added

comment:6 Changed on 07/22/2017 at 07:55:21 PM by juliandoucette

  • Blocking 5445 added

comment:7 Changed on 07/22/2017 at 07:56:22 PM by juliandoucette

  • Blocking 5444 added

comment:8 Changed on 07/22/2017 at 08:13:33 PM by kvas

  • Priority changed from P3 to P2

comment:9 Changed on 07/22/2017 at 08:52:43 PM by kvas

  • Description modified (diff)

comment:10 Changed on 07/24/2017 at 03:28:56 PM by kvas

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:11 Changed on 07/25/2017 at 03:35:05 PM by kvas

  • Description modified (diff)

comment:12 Changed on 07/26/2017 at 05:57:57 PM by kvas

  • Description modified (diff)
  • Summary changed from Create global get_page_url function in CMS to Create global get_canonical_url function in CMS

Everything but get_canonical_link is moved to #5452 (see review for the discussion of reasons).

comment:13 Changed on 08/04/2017 at 06:19:40 PM by abpbot

A commit referencing this issue has landed:
Fixes 5343 - add global function: get_canonical_url

comment:14 Changed on 08/04/2017 at 06:20:32 PM by kvas

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.