Edgewall Software
Modify

Ticket #4416 (closed defect: fixed)

Opened 5 years ago

Last modified 3 years ago

relative links are not rendered with '?' when the page doesn't exist

Reported by: ittayd@… Owned by: rblank
Priority: high Milestone: 0.11.3
Component: wiki system Version: 0.10.3
Severity: normal Keywords: relative links review
Cc:
Release Notes:
API Changes:

Description


Attachments

4416-relative-link-rendering-r7770.patch (10.2 KB) - added by rblank 3 years ago.
Patch against 0.11-stable fixing the rendering of relative links
4416-absolute-link-resolver-r7799.patch (4.4 KB) - added by rblank 3 years ago.
Fix for wiki realm special case

Download all attachments as: .zip

Change History

comment:1 Changed 5 years ago by cboos

  • Component changed from general to wiki
  • Keywords relative links added
  • Milestone set to 0.11
  • Owner changed from jonas to cboos
  • Severity changed from normal to minor

Yeah, good point, this should probably be implemented as well, for consistency.

comment:2 Changed 5 years ago by cboos

… and this raises the interesting question of the relationship between the WikiContext and the model objects, how to know if such an object exists and how to retrieve it.

comment:3 follow-up: Changed 4 years ago by cboos

  • Priority changed from normal to high
  • Severity changed from minor to normal

I should have another look at that.

Also, there's the related question of how to handle links to pages from within a sub-hierarchy. It would be quite useful to not have to bother where the page is located and have the link point to the closest page in the hierarchy.

e.g. within 0.10/TracGuide, a reference to TracBrowser will preferably point to 0.10/TracBrowser if it exists, then TracBrowser (top-level) if it doesn't.

comment:4 in reply to: ↑ 3 Changed 4 years ago by rblank

Replying to cboos:

It would be quite useful to not have to bother where the page is located and have the link point to the closest page in the hierarchy.

e.g. within 0.10/TracGuide, a reference to TracBrowser will preferably point to 0.10/TracBrowser if it exists, then TracBrowser (top-level) if it doesn't.

This is implemented in a patch attached to #4507, waiting for feedback.

comment:5 Changed 3 years ago by rblank

  • Milestone changed from 0.11-retriage to 0.11.3
  • Owner changed from cboos to rblank

I'd like to have a look at this, now that #4507 is fixed, if you don't mind.

Changed 3 years ago by rblank

Patch against 0.11-stable fixing the rendering of relative links

comment:6 Changed 3 years ago by rblank

  • Keywords review added

This turned out to be trickier than I thought, as the relative links are processed generically and not in the wiki module. The patch above fixes the rendering of relative links of the types ./... and ../.... I have not fixed the /... and //... links, as I don't think there's a reasonable way to do that.

All corner cases should be handled correctly (pages with a version number, URL arguments, a [.] link viewed in the page history, and any combinations of the above). But I'd be grateful if I could get some feedback before applying the patch.

comment:7 Changed 3 years ago by cboos

The approach looks good, I'll have more time next week to check the patch more thoroughly.

comment:8 Changed 3 years ago by cboos

I've looked at the patch a bit more in details, and it's fine by me.

comment:9 Changed 3 years ago by rblank

  • Resolution set to fixed
  • Status changed from new to closed

Thanks for the review, patch applied in [7777] (cool number ;-)

comment:10 Changed 3 years ago by rblank

  • Resolution fixed deleted
  • Status changed from closed to reopened

Mmmh, merging to trunk is tricky due to interaction with scoped links. I'll keep this ticket open to track the merge.

comment:11 Changed 3 years ago by rblank

I have merged [7777] to trunk, but I'm not really happy with the solution to the interaction with scoped links. More precisely, I'm not happy with this at all.

The trouble is, the link resolvers have no way of knowing if a target link should be absolute or relative, and the wiki link resolver resolves as scoped unless the target starts with /.

The special case for the wiki realm is ugly, but it works. A cleaner option could be to augment the link resolver interface with an additional argument that specifies if the target should be resolved as absolute. Something like 4416-absolute-link-resolver-r7799.patch. But this feels like a big change for a small issue.

Opinions?

Changed 3 years ago by rblank

Fix for wiki realm special case

comment:12 Changed 3 years ago by cboos

Give me some more time to think about it.
It's also the opportunity to address links like wiki:../other correctly (#6884).

comment:13 Changed 3 years ago by cboos

  • Milestone changed from 0.11.4 to 0.11.3

Just an update to say I didn't yet get around to this one, but it's high prio for me.
I have the feeling that the relative link should be passed as such to the resolvers, and that the wiki resolver in this case can decide to do something about it.

comment:14 Changed 3 years ago by cboos

Remy, if you don't have objections, I'd like to move this ticket to 0.11.4:

  • 0.11.3 will therefore appear clean - all tickets closed
  • I'd like to try a different approach, so I'd like to post-pone closing this ticket

comment:15 follow-up: Changed 3 years ago by rblank

Actually, this ticket *is* fixed in 0.11-stable (comment:9). The open point in comment:11 is the merge to trunk, where it interacts with wiki page scoping (only present on trunk), which I am not satisfied with.

Maybe we should close this ticket and create a new one for trunk?

comment:16 in reply to: ↑ 15 Changed 3 years ago by cboos

  • Milestone changed from 0.11.4 to 0.11.3
  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to rblank:

Maybe we should close this ticket and create a new one for trunk?

Ok, let's close this one and I'll make a note on #6884.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from rblank. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.