Opened 15 months ago

Last modified 3 months ago

#5333 new change

CMS does not support relative URLs

Reported by: juliandoucette Assignee: rhowell
Priority: P3 Milestone: Websites continuous integration
Module: Sitescripts Keywords:
Cc: wspee, kvas, jsonesen, ire, saroyanm Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29592634/

Description (last modified by wspee)

Background

When using GitLab CI for generating websites, absolute links generated by CMS become broken because all paths of the website are prefixed with an additional path component. There's no way to configure the CMS to generate relative links which would not break in such set up.

References:

What to do

To enable such deployments it should be possible to make the CMS generate relative URLs using a command line option to the generate_static_pages.py script named --relative.

Change History (17)

comment:1 Changed 15 months ago by kvas

Just like with #5334 I don't think "defect" classification is correct here. Otherwise we could make this an option to generate_static_pages.py as well. Again, I think it should be off by default since I don't think that we always want this (or want to change how production works at the moment). Sounds good?

comment:2 Changed 15 months ago by juliandoucette

  • Type changed from defect to change

Just like with #5334 I don't think "defect" classification is correct here.

Changed.

-- Again, I think the defect description is still effective.

Again, I think it should be off by default since I don't think that we always want this (or want to change how production works at the moment). Sounds good?

Yes.

comment:3 Changed 15 months ago by kvas

  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Ready set
  • Summary changed from CMS does not support relative hosts to CMS does not support relative URLs

comment:4 Changed 15 months ago by juliandoucette

I'm not sure about the "What to do" here.

I'd like a solution that provides a better way to do this that also considers this.

comment:5 Changed 15 months ago by ire

  • Cc ire added; iaderinokun removed

comment:6 Changed 14 months ago by juliandoucette

I'd like a solution that provides a better way to do ​this that also considers this.

I think I was confusing this issue for #5343 (Create global get_page_url function in CMS) when I wrote this.

comment:7 Changed 13 months ago by rhowell

  • Owner set to rhowell

comment:8 Changed 13 months ago by juliandoucette

  • Milestone set to Websites continuous integration

comment:9 Changed 11 months ago by rhowell

  • Owner rhowell deleted

comment:10 Changed 11 months ago by wspee

  • Owner set to wspee

comment:11 Changed 11 months ago by wspee

@juliandoucette

The URL mode would be controlled by a settings.ini option url-mode, which can be set to full-path (default) or relative-path (we might consider adding absolute value later to generate full absolute urls).

Do you feel strongly about this? I don't think it's necessary/useful to be able to control this via settings.ini because you'll probably never call it from command line but instead always from build automation (gulp). So unless I'm missing something here I would like to ommit this part.

comment:12 follow-up: Changed 11 months ago by juliandoucette

Do you feel strongly about this?

First priority is "whatever we have to do to make CI work". Second priority is "whatever we can do to make usage less annoying". I prefer a command line argument - that way I can use different build processes in different environments without changing the website source code.

comment:13 in reply to: ↑ 12 Changed 11 months ago by kvas

Replying to juliandoucette:

Do you feel strongly about this?

I prefer a command line argument - that way I can use different build processes in different environments without changing the website source code.

Yes, I've been also thinking that this is the best approach. I guess we can remove the part about settings.ini then.

comment:14 Changed 11 months ago by wspee

  • Description modified (diff)

Ok, changed the description accordingly. I'm not attached to the name --relative it's just what I have been using, feel free to update the description if you feel otherwise.

comment:15 Changed 5 months ago by wspee

  • Owner wspee deleted
  • Review URL(s) modified (diff)

comment:16 Changed 4 months ago by rhowell

  • Owner set to rhowell

comment:17 Changed 3 months ago by kvas

Hi Rosie,

I have looked at the changes in the reivew again as well as at the other places in the code that could be the points of intervention for implementing this ticket.

My impression is that the best and the cleanest approach would be to make the changes in converters.py around the points where resolve_link() is called: process_links() and linkify() (it seems that get_canonical_url() should return the absolute link regardless of relative/absolute URL mode).

You can start by building the tests that would fix the behavior that we want to achieve (I'm happy to advise on this, and we can also check with Julian when in doubt). After the tests are in place, we can see if there's a nice small adjustment to the link generation code that would make the tests pass. If everything fails along this path, we can think about some kind of post-processing solution along the lines of what Winsley implemented.

Cheers,
Vasily

Note: See TracTickets for help on using tickets.