Edgewall Software
Modify

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#2183 closed defect (fixed)

Deprecate and remove usge of TracLinks for embedding images

Reported by: markus Owned by: Christian Boos
Priority: low Milestone: 0.10
Component: wiki system Version: 0.9b2
Severity: minor Keywords:
Cc:
Release Notes:
API Changes:

Description

Seems like Trac doesn't like jpegs. ;-)

While source:/folder/image.jpg/ works, source:/folder/image.jpg doesn't. It's the same with source:/folder/image.jpeg/ and source:/folder/image.jpeg.

Attachments (0)

Change History (16)

comment:1 Changed 12 years ago by markus

Resolution: invalid
Status: newclosed

Just noticed that that Firefox displays the second links as plain text, while Internet Explorer displays (broken) image links. So, I guess that the ones that don't get escaped as hyperlinks are TracLinks for embedding images. Oops.

comment:2 Changed 12 years ago by markus

Well, I'm still wondering why some images (like jpg, jpeg, png) are always broken and others (like bmp) get escaped as hyperlinks.

source:trunk/setup_wininst.bmp: source:trunk/setup_wininst.bmp
source:trunk/htdocs/trac_banner.png: source:trunk/htdocs/trac_banner.png

comment:3 Changed 12 years ago by Christian Boos

Component: generalbrowser
Priority: normallow
Resolution: invalid
Severity: normalminor
Status: closedreopened

The current behavior of guessing if the source path is an image, and if so, display it, is buggy and should be phased out.

I'm reopening the ticket for that.

Plain source links should always go to the browser page for the given path.

If one wants to have an inline display of an image stored in the repository, use the Image macro and give it source:<the_path> as argument; this works fine, e.g. [[Image(source:trunk/htdocs/trac_banner.png,width=50)]]source:trunk/htdocs/trac_banner.png

Also, we should interpret links like source:<the_path>?format=raw to make a link to the raw document (see #2168).

comment:4 Changed 12 years ago by Christian Boos

Owner: changed from Jonas Borgström to Christian Boos
Status: reopenednew
  • browser.py

     
    2727from trac.wiki import wiki_to_html, wiki_to_oneliner, IWikiSyntaxProvider
    2828from trac.versioncontrol.web_ui.util import *
    2929
    30 IMG_RE = re.compile(r"\.(gif|jpg|jpeg|png)(\?.*)?$", re.IGNORECASE)
    31 
    3230CHUNK_SIZE = 4096
    3331
    3432DIGITS = re.compile(r'[0-9]+')
     
    237235                ('browser', self._format_link)]
    238236
    239237    def _format_link(self, formatter, ns, path, label):
    240         match = IMG_RE.search(path)
    241         if formatter.flavor != 'oneliner' and match:
    242             return '<img src="%s" alt="%s" />' % \
    243                    (formatter.href.file(path, format='raw'), label)
    244238        path, rev, line = get_path_rev_line(path)
    245239        if line is not None:
    246240            anchor = '#L%d' % line

comment:5 Changed 12 years ago by Christian Boos

Milestone: 0.9
Status: newassigned

comment:6 Changed 12 years ago by anonymous

Thanks for clarification, Christian. I totally agree with your first comment. The way you describe these links should work is exactly what I was expecting from Trac and but ran into problems then.

comment:7 Changed 12 years ago by Christopher Lenz

Component: browserwiki
Milestone: 0.91.0
Summary: RegEx doesn't recognize .jpg/.jpegDeprecate and remove usge of TracLinks for embedding images

I agree, but I don't think we should do this for 0.9. People/sites who've been using the image linking syntax will break.

comment:8 Changed 12 years ago by Christian Boos

It's already broken, for source: links.

source:trunk/htdocs/trac_banner.png

source:trunk/htdocs/trac_banner.png

comment:9 Changed 12 years ago by markus

Erm, yes… That's what I was trying to say. :-p

comment:10 Changed 12 years ago by Christopher Lenz

The thing is that people are probably using the syntax (using the ?format=raw parameter where necessary).

Simply removing this support now will make those embedded images simply links. Maybe that's not so bad after all?

comment:11 Changed 12 years ago by anonymous

Let's say there are people using the source: syntax (with or without ?format=raw) for embedding images. With the current version of Trac (0.9) this syntax will never lead to an embedded image inside a wiki page or ticket comment, because it simply doesn't work (it's broken for both with and without ?format=raw)

So, for existing tickets/wiki pages with this syntax in them this means that the browsers show broken image links. IMHO that's not what these people wanted to embed and so having a simple (but working) link to the TracBrowser displaying this image should be better than that.

comment:12 Changed 12 years ago by Matthew Good

Well, the reason that the "source:" image links are broken is because they point to the "/file" path, which was phased out and should now be "/browser". That should be fixed for now to keep the old behavior.

I agree that the automatic conversion of "source:" image links should be deprecated, although I think that we need to come up with some reasonable policy on how to handle deprecating features and when they will be removed.

P.S. the reason that ".bmp" links are not converted to <img> tags is because you really shouldn't be using ".bmp" files in a webpage.

comment:13 Changed 12 years ago by Christian Boos

Something like that?

  • browser.py

     
    240240        match = IMG_RE.search(path)
    241241        if formatter.flavor != 'oneliner' and match:
    242242            return '<img src="%s" alt="%s" />' % \
    243                    (formatter.href.file(path, format='raw'), label)
     243                   (formatter.href.browser(path, format='raw'),
     244                    'WARNING: deprecated form, use [[Image(%s)]] instead' \
     245                    % path)
    244246        path, rev, line = get_path_rev_line(path)
    245247        if line is not None:
    246248            anchor = '#L%d' % line

comment:14 Changed 11 years ago by Christian Boos

Milestone: 1.00.11

Deprecation warning added for milestone:0.10 in r3329, now scheduling removal for milestone:0.11.

comment:15 Changed 11 years ago by Christian Boos

Milestone: 0.110.10
Resolution: fixed
Status: assignedclosed

Actually, the removal was done in r3454. Mentioning this in the source:trunk/RELEASE for milestone:0.10 should be enough.

comment:16 Changed 11 years ago by markus

Yippie, thanks! ;-)

Modify Ticket

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