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.

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

Wrong alias root for other HTML output formats (e.g. AMP) #5760

Closed
bep opened this issue Mar 17, 2019 · 5 comments
Closed

Wrong alias root for other HTML output formats (e.g. AMP) #5760

bep opened this issue Mar 17, 2019 · 5 comments
Milestone

Comments

@bep
Copy link
Member

bep commented Mar 17, 2019

I was reading some code and test related to this, and the alias path for the AMP type of output formats looks wrong. We create redirect pages for all HTML type of Output Formats. So for:

public/page/index.html
public/amp/page/index.htm

With an alias definition of /myalias you would currently get:

public/myalias/index.html
public/myalias/amp/index.html

We have test cases for the above, but to me that looks surprising and obviously wrong.

I would expect:

public/myalias/index.html
public/amp/myalias/index.html
@bep bep added the Bug label Mar 17, 2019
@bep bep added this to the v0.55 milestone Mar 17, 2019
@bep
Copy link
Member Author

bep commented Mar 17, 2019

@regisphilibert could you have quick look at the above and tell me if you agree.

@regisphilibert
Copy link
Member

This is news to me as I'm fairly unexperienced with HTML output format with a path set, AMP included, maybe others will have more use cases to build an opinion upon.

To me though:
If we follow the AMP recommended URL structure which have you recreate a sort of "alternate" site, yes, the correction you want tot make is needed.
But for any other HTML output format, the current behaviour is, I think, adapted to the user's expectation.

If this correction is limited to AMP, then I agree, if this is for every HTML output formats with a path set, then no I don't agree.

@regisphilibert
Copy link
Member

I just found out that the path setting's value is added at the root fo the URL for every HTML (or isHtml = true) output formats.

I was not understanding the path setting properly.

@bep
Copy link
Member Author

bep commented Mar 18, 2019

If this correction is limited to AMP, then I agree, if this is for every HTML output formats with a path set, then no I don't agree.

This is relevant for all HTML output formats with PATH set (which is kind of very AMP specific).

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants