Edgewall Software
Modify

Opened 14 years ago

Closed 12 years ago

#4416 closed defect (fixed)

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

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

Description


Attachments (2)

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

Download all attachments as: .zip

Change History (18)

comment:1 by Christian Boos, 14 years ago

Component: generalwiki
Keywords: relative links added
Milestone: 0.11
Owner: changed from Jonas Borgström to Christian Boos
Severity: normalminor

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

comment:2 by Christian Boos, 14 years ago

… 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 by Christian Boos, 13 years ago

Priority: normalhigh
Severity: minornormal

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.

in reply to:  3 comment:4 by Remy Blank, 12 years ago

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 by Remy Blank, 12 years ago

Milestone: 0.11-retriage0.11.3
Owner: changed from Christian Boos to Remy Blank

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

by Remy Blank, 12 years ago

Patch against 0.11-stable fixing the rendering of relative links

comment:6 by Remy Blank, 12 years ago

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 by Christian Boos, 12 years ago

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

comment:8 by Christian Boos, 12 years ago

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

comment:9 by Remy Blank, 12 years ago

Resolution: fixed
Status: newclosed

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

comment:10 by Remy Blank, 12 years ago

Resolution: fixed
Status: closedreopened

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

comment:11 by Remy Blank, 12 years ago

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?

by Remy Blank, 12 years ago

Fix for wiki realm special case

comment:12 by Christian Boos, 12 years ago

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

comment:13 by Christian Boos, 12 years ago

Milestone: 0.11.40.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 by Christian Boos, 12 years ago

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 by Remy Blank, 12 years ago

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?

in reply to:  15 comment:16 by Christian Boos, 12 years ago

Milestone: 0.11.40.11.3
Resolution: fixed
Status: reopenedclosed

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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.