Edgewall Software
Modify

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#11009 closed defect (fixed)

Bold WikiFormatting leaking outside ticket body

Reported by: Andrew C. Martin <andrew.c.martin@…> Owned by: Peter Suter
Priority: high Milestone: 0.12.6
Component: wiki system Version:
Severity: major Keywords: formatter
Cc: andrewcmartin@…, Jun Omae Branch:
Release Notes:

Fix bad html produced when a binary markup tag (e.g. **) immediately follows a wiki list.

API Changes:

Description (last modified by Christian Boos)

Having the following text in a ticket will result in all subsequent ticket description, comments, and workflow text to default to a bold font weight. The WikiFormatting should not extend past the scope in which it is started.

***
* 
***

Note there is one space on the second line.

I've verified this bug in Trac 0.12.3 and Trac 1.0.1dev-r11320


For a demo, see ticket:11009?version=4 (I had to "fix" the description, as this prevented making further changes, at least with Chrome, see comment:5). — cboos

Attachments (1)

T11009-list-wikiformat-leak.patch (4.2 KB ) - added by Peter Suter 5 years ago.
Close list before checking open tags.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by Andrew C. Martin <andrew.c.martin@…>

This comment does not have any bold WikiFormatting.

comment:2 Changed 6 years ago by Andrew C. Martin <andrew.c.martin@…>

This comment does have bold WikiFormatting.

comment:3 Changed 6 years ago by Andrew C. Martin <andrew.c.martin@…>

Description: modified (diff)

comment:4 Changed 6 years ago by Peter Suter

Component: renderingwiki system
Keywords: formatter added
Milestone: topic-wikiengine

Seems like a list item immediately followed by some opening tag is broken. Closing the list flushes all tags including the closing tag corresponding to the opening tag yet to be written. So this also affects e.g. ordered lists and italic text:

1. The list item
//The closing tag that should follow this opening tag was flushed out when closing the list

results in:

<ol><li>The list item
</em></li></ol></p>
<em>The closing tag that should follow this opening tag was flushed out to close the list
</p>

comment:5 Changed 6 years ago by Christian Boos

Description: modified (diff)

Italic as well, of course, as still visible in ticket:11013?version=0.

And somehow this propagates to breaking the dynamic preview, and sometimes prevent updating the ticket (I couldn't save my change to #11013 until I fixed the formatting of the description, due to the "someone else modified the ticket in the meantime" error). I suppose it's simply due to the broken HTML structure we end up with.

Ah, and same thing here, now.

comment:6 Changed 6 years ago by Christian Boos

Description: modified (diff)
Priority: normalhigh
Severity: normalmajor

comment:7 Changed 5 years ago by Jun Omae

Cc: Jun Omae added

#11373 was closed as duplicate. Also, proposed changes can be found in log:jomae.git:ticket11009_0.12.6dev.

comment:8 in reply to:  7 ; Changed 5 years ago by Jun Omae

#11373 was closed as duplicate. Also, proposed changes can be found in log:jomae.git:ticket11009_0.12.6dev.

Could someone review the patch?

Changed 5 years ago by Peter Suter

Close list before checking open tags.

comment:9 in reply to:  8 ; Changed 5 years ago by Peter Suter

Replying to jomae:

log:jomae.git:ticket11009_0.12.6dev.

Could someone review the patch?

Looks almost good to me.

But I think _indirect_tag_handler should first close any lists if needed and only then check if the tag is already open. Otherwise the following fails:

 1. //italic in list
//italic
in paragraph

With your patch _indirect_tag_handler:

  1. First checks if the // tag is open (yes)
  2. Then closes the list (this flushes and closes the open // tag)
  3. Attempts to close the // tag again (because the check in 1. said yes).

Fixing this actually simplifies the code. I added this testcase, the simplified _indirect_tag_handler and a similar simplification of _bolditalic_formatter to your code in the attached patch.

Last edited 5 years ago by Peter Suter (previous) (diff)

comment:10 Changed 5 years ago by Peter Suter

Milestone: topic-wikiengine0.12.6

comment:11 in reply to:  9 Changed 5 years ago by Jun Omae

Fixing this actually simplifies the code. I added this testcase, the simplified _indirect_tag_handler and a similar simplification of _bolditalic_formatter to your code in the attached patch.

Thanks for reviewing. Your patch looks pretty good rather than my patch. Please commit!

comment:12 Changed 5 years ago by Peter Suter

Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Committed in [12549] and merged in [12550-12551].

comment:13 Changed 5 years ago by Peter Suter

Owner: set to Peter Suter

Modify Ticket

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