why not use: https://github.com/adobe/helix-html-pipeline/blob/main/src/steps/utils.js#L184

also, we need to be careful not to change the behaviour of the canonical url until we decided how it should be handled.

The new code only makes sure no ref--repo--owner.hlx.page|live appears in meta on production, it does not change the canonical computation logic.

We could reuse rewriteUrls here. That util is also used in the rewrite-urls step and turns URLs like this into path-only URLs. Or do we really need absolute URLs here?

Or do we really need absolute URLs here?

or add a flag makeAbsolute to rewrite-urls()

also, it would probably be better to apply the rewriting in render, as the logic in metadata is already a bit complex:

meta are strings only (not anchor with href), I think it must stay absolute urls - you would need a special treatment to consume the meta if it can be a relative or an absolute url (if external link). In fact, making them relative might break all existing logic...

I initially started with rewriteUrls but several things:

1. it does not make absolute urls but relative - the extra makeAbsolute flag would be required and this adds extra logic which is currently isolated in "my" makeAbsoluteURLForMeta function.
2. it does not deal with branches - if an author page a url with custombranch--repo--owner.hlx.page, it is not considered - this could be an improvement of the current method
3. because rewriteUrls is used in different places, I did not want to interfere and take the risk to change some behaviour

I can re-try with rewriteUrls and see the impact.

also, it would probably be better to apply the rewriting in render, as the logic in metadata is already a bit complex:

helix-html-pipeline/src/steps/render.js

Lines 62 to 72 in c7b98c4

	for (const [name, value] of Object.entries(meta.page)) {
	const attr = name.includes(':') && !name.startsWith('twitter:') ? 'property' : 'name';
	if (Array.isArray(value)) {
	for (const v of value) {
	appendElement($head, createElement('meta', attr, name, 'content', v));
	}
	} else {
	appendElement($head, createElement('meta', attr, name, 'content', value));
	}
	}
	appendElement($head, createElement('link', 'rel', 'alternate', 'type', 'application/xml+atom', 'href', meta.feed, 'title', `${meta.title} feed`));

But I assume we do not get the values from the metadata sheet in here... no ?

dev branch for testing can be easily achieved in the client side code consuming the metadata: it needs the path only anyway and use the current host. And it is already the case today: the code cannot assume it gets the correct host anyway if the content is managed by an author.
The only problem that this solves is authors copy/pasting urls from any of the 3 hosts in the metadata of their Word / gdoc documents, as we tell them to do. If they paste a hlx.page url, we should just make sure it does not appear in production. All the rest is anyway out of scope and does to change here.

And I do not think this is a "BREAKING CHANGE", slight change in behaviour, yes, but you have a URL today, you still get a URL after this. Different host, yes but points to same "content" in prod.

extra logic which is currently isolated in "my" makeAbsoluteURLForMeta function.

Ok, switching to relative links may be too breaking, but isn't similar behavior alreasy in makeCanonicalHtmlUrl today? I wouldn really try to reuse existing rewriting logic as much as possible.

Nope. makeCanonicalHtmlUrl is doing something else (only appending index.html where needed).
We have a large collection of methods doing similar but different things. That's why it is hard to plug on top without breaking backward compatibility.
Also rewriteUrl should be named makeRelative because this is really what it is doing.

And I do not think this is a "BREAKING CHANGE",

yes it is. projects might expect .hlx.page or hlx.live urls, and might be not be able to deal with the production url.