Edgewall Software

Ticket #2183 (closed defect: fixed)

Opened 3 years ago

Last modified 2 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:

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

Changed 3 years ago by markus

  • status changed from new to closed
  • resolution set to invalid

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.

Changed 3 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

Changed 3 years ago by cboos

  • priority changed from normal to low
  • resolution invalid deleted
  • status changed from closed to reopened
  • component changed from general to browser
  • severity changed from normal to minor

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

Changed 3 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 

Changed 3 years ago by cboos

  • status changed from new to assigned
  • milestone set to 0.9

Changed 3 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.

Changed 3 years ago by cmlenz

  • summary changed from RegEx doesn't recognize .jpg/.jpeg to Deprecate and remove usge of TracLinks for embedding images
  • component changed from browser to wiki
  • milestone changed from 0.9 to 1.0

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.

Changed 3 years ago by cboos

It's already broken, for source: links.

source:trunk/htdocs/trac_banner.png

source:trunk/htdocs/trac_banner.png

Changed 3 years ago by markus

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

Changed 3 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?

Changed 3 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.

Changed 3 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.

Changed 3 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 

Changed 3 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.

Changed 2 years ago by cboos

  • status changed from assigned to closed
  • resolution set to fixed
  • milestone changed from 0.11 to 0.10

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

Changed 2 years ago by markus

Yippie, thanks! ;-)

Add/Change #2183 (Deprecate and remove usge of TracLinks for embedding images)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
to The owner will change from cboos. Next status will be 'closed'
 
Note: See TracTickets for help on using tickets.