Edgewall Software
Modify

Opened 6 years ago

Closed 4 years ago

#10751 closed defect (fixed)

Image macro now shows up in inline wiki markup

Reported by: Christian Boos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.5
Component: wiki system Version: 1.0dev
Severity: normal Keywords: image macro
Cc: Ryan J Ollos
Release Notes:

The Image macro no longer renders inline XHTML elements by default. This avoids the appearance of images in the timeline, wiki section headings and other oneliner content. Inline XHTML elements can be explicitly specified using the inline keyword as a macro argument.

API Changes:

Description (last modified by Christian Boos)

Caught today live on t.e.o: snapshot from timeline:2012-06-28

Not that good, especially for big images like this.

Attachments (2)

timeline-image.png (30.3 KB ) - added by Christian Boos 6 years ago.
snapshot from timeline:2012-06-28
timeline-image-afterpatch.png (30.5 KB ) - added by Ryan J Ollos <ryan.j.ollos@…> 6 years ago.

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by Christian Boos

Attachment: timeline-image.png added

snapshot from timeline:2012-06-28

comment:1 Changed 6 years ago by Christian Boos

Description: modified (diff)

png!

comment:2 in reply to:  1 Changed 6 years ago by Christian Boos

Ok, so currently is_inline is unconditionally True … maybe we should return True only if explicitly specified, that way if you really want to have a picture in, say, a section title, you'd have to add the inline parameter.

comment:3 Changed 6 years ago by Christian Boos

Milestone: 1.0next-stable-1.0.x

comment:4 Changed 6 years ago by flyhigh

Same problem with JOSM Trac #8683

comment:5 Changed 6 years ago by flyhigh

This happens if the Image macro is used within the first two lines of a ticket/comment.

Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

comment:6 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

Cc: ryan.j.ollos@… added

Replying to cboos:

Ok, so currently is_inline is unconditionally True … maybe we should return True only if explicitly specified, that way if you really want to have a picture in, say, a section title, you'd have to add the inline parameter.

I did some experimenting and it seems like a good solution. There was a similar issue for Bloodhound's activity feed (bh:#279), which was temporarily worked-around by using the CSS display: none for img elements. The solution presented in this ticket seems like a better solution, at least for the timeline.

I worked in t10751, adding another parameter to parse_args function. I considered calling trac.util.text.stripws in parse_args, but was unsure about this. I made an attempt at refactoring ImageMacro.expand_macro to use parse_args, which is the reason for the additional Image macro tests in the branch. The refactoring didn't work out - we need strict = True in parse_args to prevent a pathspec containing a = character from being interpreted as a kw argument, but with strict = True, a parameter such as margin-left=10 isn't interpreted as a kw argument. I didn't see a way to solve that issue without making the code more complex than before the start of the refactoring, so I abandoned it.

comment:7 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

It looks like I have an error in the comment for parse_args: :param: strict should be :param strict:. I'll wait for feedback in case other changes are needed before fixing that.

comment:8 Changed 6 years ago by Ryan J Ollos <ryan.j.ollos@…>

Just spotted a place on the t.e.o site where there is an image in a section heading: see Clients. The patch proposed here would result in the need to edit that wiki markup to add the inline parameter. That makes me think the change should be targeted for a 1.1.x milestone rather than a 1.0.x milestone.

comment:9 Changed 5 years ago by Ryan J Ollos

Cc: Ryan J Ollos added; ryan.j.ollos@… removed

comment:10 Changed 5 years ago by Ryan J Ollos

Keywords: macro added

comment:11 Changed 4 years ago by Ryan J Ollos

Milestone: next-stable-1.0.x1.1.4
Owner: changed from Christian Boos to Ryan J Ollos
Status: newassigned

comment:12 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)

Proposed changes in log:rjollos.git:t10751.2.

comment:13 Changed 4 years ago by Jun Omae

Looks good to me. I think it would lead no problem to apply it to 1.0-stable….

comment:14 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.41.0.5

Thanks, I'll apply to 1.0-stable. I was on the fence about that anyway.

comment:15 Changed 4 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

Committed to 1.0-stable in [13791:13793], merged to trunk in [13794:13796].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos 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.