Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.44k stars 1.99k forks source link

Site Editor & Blocks: Navigation and other page links aren't relative anymore & can break with changes to primary domain #94034

Open supernovia opened 3 months ago

supernovia commented 3 months ago

Quick summary

Right now when someone changes their site's primary address, links in (site editor) site navigation, (block) buttons, and (block based) gallery items are not updated, and will continue to point to the old address. This can cause the site to break.

As far as I'm aware, this has not always been the case: in the past, unless users hard-coded the links themselves (usually copy-pasting a page link instead of using the page link tool) the site would gracefully update navigation and other links to a new address. I don't know whether they used relative links or whether our system used to just add the correct primary site link when generating the html, but I don't remember navigation and other links like this breaking with URL changes before.

https://github.com/user-attachments/assets/cda9bf34-a325-4bfd-a7fd-17983aa8169f

Steps to reproduce

  1. Start by connecting a custom domain with a simple site.

  2. Add a few things: custom navigation linking to specific pages, a gallery with images that link to attachment pages, and a button that links to the contact page. You might notice the links appear to show relative urls, like /contact, which should work with an address change. Screenshot 2024-08-29 at 2 03 49 PM

  3. Publish your work

  4. Disconnect the custom domain, so the site is on its free address.

You could also start with one free address, then rename to some other free address.

What you expected to happen

I would expect navigation, buttons, and media links within the site that are not intentionally hard-coded to link to the current primary domain, so that the site will continue to work even if the user changed the site address.

What actually happened

Any navigation item, image with attachment page link, or button link within the site that was set up with the old address will stay on the old address, thus breaking the site if the old name stops working.

Impact

Some (< 50%)

Available workarounds?

Yes, difficult to implement

If the above answer is "Yes...", outline the workaround.

Search all posts, pages, and templates within the site for the old address, and replace it by hand. Or, if we have access to a find-and-replace script we could update that way.

If on Atomic, a CLI search replace like this can help (be sure to remove dry run after confirming it's correct) search-replace "olddomain.wpcomstaging.com" "newdomain.com" --skip-columns=guid --dry-run

Platform (Simple and/or Atomic)

Simple (including transfers from atomic) Atomic (I haven't tested but did run into a case today; note @arthur791004 tested and wasn't able to replicate it on atomic)

Logs or notes

I didn't have time to test in Atomic, but it is less of an issue there because we do have find/replace options available to swap old addresses for new.

supernovia commented 3 months ago

I had a hard time describing this, so if you have a better title, please add it! Here is one case from today, where the navigation links broke when they changed the site address: 8667693-zd-a8c

supernovia commented 3 months ago

Here is another case: they lost their old domain, but it's hard-coded into the attachment-page links in their gallery block. 8667693-zd-a8c

This one seems to show the problem has been around for a while, and perhaps I was just "lucky" to run into two cases within an hour, but at any rate I'm guessing this could be a rough experience for people who change names for any reason (consider folks connecting their domain to launch a site redesign, for example - I've not had time to test that but I'm guessing it could be an issue.)

github-actions[bot] commented 3 months ago

Support References

This comment is automatically generated. Please do not edit it.

retnonindya commented 3 months ago

📌 REPRODUCTION RESULTS

I wanted to test on self-host, but setting up primary custom domain for my self-host site takes quite some time.

📌 FINDINGS/SCREENSHOTS/VIDEO I can replicate.

Screenshot 2024-09-04 at 3 33 40 PM

📌 ACTIONS

Considering the effect when the user changes the primary address to the Navigation Block and how it affects Simple sites with minimum/no workaround available, I'm setting this to High/Blocker.

supernovia commented 2 months ago

In this case someone's menu links were broken after a revert because they pointed to a temporary address. :/ I'm not sure how that happened, but it seems having relative URLs or URLs based on the site address would help. 8662919-zd-a8c

supernovia commented 2 months ago

Since this affected a site transfer for a revert, I'm wondering if it affects sites that are going being synced from staging to production, too; we should probably find out!

arthur791004 commented 2 months ago

Tested on atomic sites and the link will be updated when you change the primary address.

supernovia commented 2 months ago

Tested on atomic sites and the link will be updated when you change the primary address.

I just ran into a case on Atomic where they changed primary domains and everything broke as links weren't updated, so I'll add Atomic back to this case

arthur791004 commented 2 months ago

Here is the related discussion: p1726027362071599-slack-C03N25JPCE4. The solution seems to add the hook to do search-replace command on simple sites when the primary domain changes.

mrfoxtalbot commented 2 months ago

@arthur791004, do you if fixing https://github.com/Automattic/wp-calypso/issues/94360 will fix this issue too? I want to make sure this gets addressed quickly. Thanks!

arthur791004 commented 2 months ago

do you if fixing https://github.com/Automattic/wp-calypso/issues/94360 will fix this issue too?

No. The current solution for https://github.com/Automattic/wp-calypso/issues/94360 focuses on the URL of images but we can propose a simliar solution for all links.

mrfoxtalbot commented 2 months ago

Got it. I see you unassigned yourself @fushar. Is there anyone else who could take care if this? It's clearly high-priority.

arthur791004 commented 2 months ago

I unassigned myself as I want to hand over to the next groundkeepers

arthur791004 commented 2 months ago

Proposing a possible workaround, D161437-code, but it doesn't solve the problem completely.

candy02058912 commented 2 months ago

Moving this issue to "Needs shaping" as we realized this is closely related to https://github.com/Automattic/wp-calypso/issues/94360 and would be better to solve together.

donnapep commented 1 month ago

@arthur791004 Given you started working on a solution, could this issue be moved from The One Board to your team's board?

arthur791004 commented 1 month ago

Okay.

lsl commented 3 weeks ago

@arthur791004 I don't know if that approach in D161437-code will work given domains can move or be deleted. I think we'll be better off running something like the image fixer p7DVsv-lsR-p2 on primary domain change.

cc @daledupreez in case you want to extend the fixer tool to support more blocks and maybe run it in an async job on domain change?

@supernovia You can get relative links in these buttons but the default seems to be to store the domain along with it. Not sure if that's always been the case or not but I know it's been this way for quite a while in core.

https://github.com/user-attachments/assets/d5cd5874-2735-46e9-8a64-4dc97853f402

About page selected Image

/about saved Image

lsl commented 3 weeks ago

Added this to dotcom projects opportunity inbox. Not sure if it needs a project or not but there needs to be some kind of shaping and definition around what we want to solve here. - It's not something groundskeeping is suited to solve.

jartes commented 3 weeks ago

Another user with a simple site here: 9015701-zd-a8c

They change their site domain, and their navigation links from the menu are pointing to the old domain.

arthur791004 commented 2 weeks ago

We have made a wpcom-search-replace CLI tool on simple sites. I'll continue to check the sites listed in this issue and run the CLI tool to fix it tomorrow.

supernovia commented 2 weeks ago

9057624-zd-a8c

supernovia commented 2 weeks ago

@arthur791004 thank you! Just to clarify, is the underlying issue now solved and we just need to fix any sites that are broken, or should we watch for new cases and report them here til this is closed?

arthur791004 commented 2 weeks ago

@supernovia It would be better to report them here for now. If there are lots of reports, we'll consider to propose another fix to replace links with current primary domain automatically.

9057624-zd-a8c

I assume this is a great example that we didn't do it. Since users can see the links on both the Editor and frontend, they might feel confused if we replace the domain of link on the fly as follows:

As a result, we're considering to how to eliminate this confusing before applying the replacement automatically.

arthur791004 commented 2 weeks ago

I've checked the tickets in this issue, and run the CLI tools to fix the sites that still encountered the problem. Here is the latest update - p58i-ivB-p2#comment-65160.

supernovia commented 13 hours ago

Hi @arthur791004 , we've got another one here. Let's check with @radtechgh before replacing any URLs though. 9125465-zd-a8c