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
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
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: ↓ 4 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
- Attachment 4416-relative-link-rendering-r7770.patch added
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
- Attachment 4416-absolute-link-resolver-r7799.patch added
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
comment:15 follow-up: ↓ 16 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



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