Edgewall Software
Modify

Opened 12 years ago

Closed 10 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 Branch:
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:
Internal 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 12 years ago.
snapshot from timeline:2012-06-28
timeline-image-afterpatch.png (30.5 KB ) - added by Ryan J Ollos <ryan.j.ollos@…> 12 years ago.

Download all attachments as: .zip

Change History (17)

by Christian Boos, 12 years ago

Attachment: timeline-image.png added

snapshot from timeline:2012-06-28

comment:1 by Christian Boos, 12 years ago

Description: modified (diff)

png!

in reply to:  1 comment:2 by Christian Boos, 12 years ago

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 by Christian Boos, 12 years ago

Milestone: 1.0next-stable-1.0.x

comment:4 by flyhigh, 12 years ago

Same problem with JOSM Trac #8683

comment:5 by flyhigh, 12 years ago

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

by Ryan J Ollos <ryan.j.ollos@…>, 12 years ago

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

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 by Ryan J Ollos <ryan.j.ollos@…>, 12 years ago

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 by Ryan J Ollos <ryan.j.ollos@…>, 12 years ago

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 by Ryan J Ollos, 11 years ago

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

comment:10 by Ryan J Ollos, 11 years ago

Keywords: macro added

comment:11 by Ryan J Ollos, 10 years ago

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

comment:12 by Ryan J Ollos, 10 years ago

Release Notes: modified (diff)

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

comment:13 by Jun Omae, 10 years ago

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

comment:14 by Ryan J Ollos, 10 years ago

Milestone: 1.1.41.0.5

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

comment:15 by Ryan J Ollos, 10 years ago

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. Next status will be 'reopened'.
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.