Edgewall Software
Modify

Opened 17 years ago

Closed 16 years ago

Last modified 11 years ago

#5968 closed defect (fixed)

InterTrac links with blank target don't work even though TracLinks of the same form do

Reported by: ben@… Owned by: Christian Boos
Priority: normal Milestone: 0.11.1
Component: wiki system Version:
Severity: normal Keywords: intertrac
Cc: kevin@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by Christian Boos)

Inside a wiki page [wiki: blah] works to link to the top level of the wiki; however, [trac:wiki: blah] doesn't work (I get a '"wiki:" is not a TracLinks' error).

If I turn intertrac compat mode, then the link works (because the originating Trac instance formats the link).

Attachments (1)

t5968.patch (3.4 KB ) - added by Christian Boos 17 years ago.
Same change than the inlined diff, plus some tests and a cosmetic change for the link title when there's no target.

Download all attachments as: .zip

Change History (12)

comment:1 by Christian Boos, 17 years ago

Component: generalwiki
Keywords: intertrac added
Milestone: 0.11
Owner: changed from Jonas Borgström to Christian Boos
Status: newassigned

Suggesting:

  • trac:wiki or [trac:wiki] for linking to the Wiki module
  • [trac:] for linking to the toplevel (#4314)

Which can be implemented by removing a few checks in the intertrac module:

  • trac/wiki/intertrac.py

     
    4141
    4242    def process_request(self, req):
    4343        link = req.args.get('link', '')
    44         if not link:
    45             raise TracError('No TracLinks given')
    4644        link_elt = extract_link(Context(self.env, req), link)
    4745        if isinstance(link_elt, Element):
    4846            href = link_elt.attrib.get('href')
    49             if href:
    50                 req.redirect(href)
    51         raise TracError('"%s" is not a TracLinks' % link)
     47        else:
     48            href = req.href(link)
     49        req.redirect(href)
    5250
    53 
    5451    # IWikiMacroProvider methods
    5552
    5653    def get_macros(self):

Invalid InterTrac links will instead trigger a "No handler matched request to <link>" error.

by Christian Boos, 17 years ago

Attachment: t5968.patch added

Same change than the inlined diff, plus some tests and a cosmetic change for the link title when there's no target.

comment:2 by Christian Boos, 17 years ago

Resolution: fixed
Status: assignedclosed

Patch applied in [6137].

comment:3 by Kevin Gabbert, 17 years ago

Resolution: fixed
Status: closedreopened

I'm having this same exact problem in .11rc1 (windows). The most recent download at this time

was there a regression?

the exact error I get is:

Internal Error

"wiki:" is not a TracLinks

comment:4 by kevin@…, 17 years ago

Cc: kevin@… added

comment:5 by Christian Boos, 17 years ago

Description: modified (diff)

What is currently implemented is the suggestion presented in comment:1, i.e. [<intertrac-prefix>:wiki].

However, I agree that [<intertrac-prefix>:wiki:] could be normalized to the same target.

I use this opportunity to add a reminder about another problem with InterTrac links:

[trac:SpamFilter SpamFilter],
[trac:SpamFilter the SpamFilter]

renders as:

(while the second works as expected, the first also shows the trac: prefix when it shouldn't)

in reply to:  5 ; comment:6 by osimons, 17 years ago

Replying to cboos:

(while the second works as expected, the first also shows the trac: prefix when it shouldn't)

That would be me in [6417], and it only occurs when the target and label are identical. Problem is that the _make_link() code does not know the differenence as the _lhref_formatter() sets label to target if no label is set before passing on to rendering.

The reason for wanting the prefix was to treat intertrac/wiki links like any other external link like http:// and similar. The default behaviour without label should include the external target.

I haven't looked into what that match argument passed in to _make_link() actually contains, but if that allows me to identify 'no label' it would just be a matter of changing if label == target: to something looking into the match object instead (formatter.py).

in reply to:  6 comment:7 by osimons, 17 years ago

Replying to osimons:

Replying to cboos:

(while the second works as expected, the first also shows the trac: prefix when it shouldn't)

That would be me in [6417], and it only occurs when the target and label are identical. Problem is that the _make_link() code does not know the differenence as the _lhref_formatter() sets label to target if no label is set before passing on to rendering.

Seems match contains the full wiki link text. Using some string trickery, it should then be possible to detect if there is no actual label set when the strings are identical. This works when testing your examples:

  • trac/wiki/formatter.py

     
    390390            else:
    391391                return olabel or otarget
    392392        else:
    393             if label == target:
     393            if label == target \
     394                    and match[match.find(label):].rstrip(']') == label:
    394395                # add ns for Inter* links when nothing is set
    395396                label = ns+':'+label
    396397            return self._make_intertrac_link(ns, target, label) or \

comment:8 by Christian Boos, 17 years ago

What about passing the fullmatch object to _make_link and look for the 'label'group? If present, then always use it, otherwise use ns:label because it will be either a shref or a lhref with no label set. That would be cleaner I think.

in reply to:  8 comment:9 by osimons, 17 years ago

Replying to cboos:

What about passing the fullmatch object to _make_link

Yup. Makes more sense than trying to reparse it again. Committed in [7238:7239] for 0.11 and trunk.

That concludes the detour from the main ticket topic, I guess.

comment:10 by Christian Boos, 16 years ago

Resolution: fixed
Status: reopenedclosed

Last fix done in [7312].

comment:11 by Christian Boos, 15 years ago

I'd like to renegotiate comment:6 ;-) e.g. [trachacks:ThatPlugin] displaying the same as trachacks:ThatPlugin, i.e. trachacks:ThatPlugin instead of ThatPlugin.

Your reasoning was that trachacks: prefix should be shown always, like we do for [http://...], but I think that Inter* prefixes are more like realm prefixes and that we should hide them when seen inside [] brackets. http: is special cased because of the target.startswith('//') test, for detecting whether the ns prefix is a communication protocol or not, obviously not the case here.

After all, one can always write a plain trachacks:ThatPlugin string when one wants to see that Inter* prefix. With the current r6417 / r7239 behavior, we have no way of hiding the prefix (except from the heavy handed [trachacks:ThatPlugin ThatPlugin]).

So I'd like to revert this, especially now that the hack I suggested to you in comment:8 now gets in my way for reusing Formatter._make_link in another regexp …

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos 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.