Edgewall Software
Modify

Ticket #2183 (closed defect: fixed)

Opened 6 years ago

Last modified 5 years ago

Deprecate and remove usge of TracLinks for embedding images

Reported by: markus Owned by: cboos
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

Change History

comment:1 Changed 6 years ago by markus

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

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 6 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 6 years ago by cboos

  • Component changed from general to browser
  • Priority changed from normal to low
  • Resolution invalid deleted
  • Severity changed from normal to minor
  • Status changed from closed to reopened

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 6 years ago by cboos

  • Owner changed from jonas to cboos
  • Status changed from reopened to new
  • 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 6 years ago by cboos

  • Milestone set to 0.9
  • Status changed from new to assigned

comment:6 Changed 6 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 6 years ago by cmlenz

  • Component changed from browser to wiki
  • Milestone changed from 0.9 to 1.0
  • Summary changed from RegEx doesn't recognize .jpg/.jpeg to Deprecate 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 6 years ago by cboos

It's already broken, for source: links.

source:trunk/htdocs/trac_banner.png

source:trunk/htdocs/trac_banner.png

comment:9 Changed 6 years ago by markus

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

comment:10 Changed 6 years ago by cmlenz

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 6 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 6 years ago by mgood

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 6 years ago by cboos

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 6 years ago by cboos

  • Milestone changed from 1.0 to 0.11

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

comment:15 Changed 6 years ago by cboos

  • Milestone changed from 0.11 to 0.10
  • Resolution set to fixed
  • Status changed from assigned to closed

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

comment:16 Changed 6 years ago by markus

Yippie, thanks! ;-)

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 cboos. 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.