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

Fix #1614 Rendering of link color doesn't update until page with the link is saved again #7367

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sergeus
Copy link

@Sergeus Sergeus commented Sep 29, 2024

This change is a fix for #1614

After this change, the following scenarios work:

  • Creating a page now causes links to its path to be immediately updated to be valid (red -> blue)
  • Deleting a page now causes links to its path to be immediately updated to be invalid (blue -> red)
  • Moving a page to a path now causes existing links to the new path to be valid (red -> blue)

All of the above now also work for links that do not specify a locale (e.g. /path/to/page instead of /en/path/to/page). The old logic only worked for links that did specify a locale. As far as I can tell, paths without a locale redirect to the wiki's configured default locale and that's explicitly supported behavior. I've kept this change in line with that behavior - only default-locale-pages cause the state of links that lack a locale to be updated.

However, one scenario remains and I haven't been able to fix this one (more detail below):

  • Moving a page from a path still leaves existing links to the old path marked as valid (they do not change)

I think this change is still a good one even with that drawback though. In the current state of the repo, all 4 scenarios don't work, and now 3/4 do. (Plus the non-locale links as well!) However, if you know where I should look to fix the final scenario, I'd be happy to do that.

Tech details
There are 2 problems that I fixed, and 1 additional one that still remains. The logic was already in the codebase to try to handle this, but some small bugs prevented it from working.

Problem 1
Finding the links in existing pages never did any replacements.

This seems to be an ordering problem - the html attributes for class and href in the string that reconnectLinks was using were the other way around. Interestingly, Chrome's debugging tools show the same string as reconnectLinks used to use, but Firefox's shows the attributes reversed. I've tested both orders and Firefox's is definitely the one that works, regardless of which browser you use to create the pages involved.

Problem 2
Links with no locale prefix. Like I mentioned above, the old reconnectLinks logic only operates on links with a locale prefix. I've made this a double pass - so it does the existing logic first (for links with locale prefixes) and if the locale matches the wiki's default locale, it repeats that process for links that don't have a locale prefix.

Problem 3
Moving a page doesn't update the links that point to its old path. I get the impression from the code that it's supposed to do this, but this specific scenario definitely does not work for me, even with my fix. The strings it is trying to replace definitely match the page, but the replacements don't happen. I can only guess that something in the process of moving the page is invalidating some data about its old path that means it's not able to find the referencing pages correctly, so even though the intended replacement is right, it doesn't actually execute it.

This scenario is also a bit deeper, because if the link is supposed to be updated live, my understanding of this html replacement logic is that it's modifying only the rendered page, not the source (Markdown or whatever the user chose), and the source would also need to be updated for this behavior to work right. And from what I can see, that doesn't happen either, so updating only the generated html would be very confusing, because then the link would go back to being broken the next time the page was regenerated. (Unless I'm misunderstanding the layer of data that code is operating on!)

…re-render for page creations, deletions, and moves.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants