Comment TracLinks without a ticket id should not be shown in strikethrough

[Ryan J Ollos]

As originally discussed in ticket:9981, TracLinks within a ticket such as comment:1 should not be shown in strike-through font. Links pointing to other tickets, such as comment:1ticket:9981, should be shown in strike-through font, which was the intent of #9981.

[Ryan J Ollos <ryano@…>]

The patch attempts to resolve a couple of issues:

1. TracLinks of the form comment:N are no longer shown in strike-through font when the enclosing ticket is closed (and neither are TracLinks of the form comment:N:ticket:M, when present in ticket M).
2. Possible error in #9981: class_=status was added to the HTML generated for TracLinks in the comment realm, but that was inconsistent with class_='%s ticket' % status that is applied in _format_link to the HTML generated for TracLinks in the ticket realm. It doesn't appear this presented any immediate problems.
3. Possible error in #9981: The functional test added in [10643] was expecting a link with class closed ticket, but the test was still passing despite issue (2) because it was testing the markup ticket:%s#comment:1, which was actually testing a code pathway through _format_link, not _format_comment_link.

I added and modified some tests in trunk/trac/ticket/tests/wikisyntax.py​. As best I could determine though, without significant modifications to trunk/trac/wiki/tests/formatter.py​ it would not be possible to add a test for a comment:N link in wikisyntax.py since the tests are setup and executed in a wikipage. The comment here​ might be relevant, but I don't understand it.

Here are some issues I'm considering addressing in a follow-on patch:

4. TracLinks of the form comment:N:ticket:M should display with class missing ticket when ticket M does not exist or the user doesn't have permission to view ticket M. For example, compare: ticket:100000 vs comment:1:ticket:100000 - the latter should display as a missing link.
5. comment:N and comment:N:ticket:M should display with class missing comment when ticket M does exist and the user has permission to view ticket M, but comment N does not exist. For example, compare: comment:1 vs comment:100000 and comment:1:ticket:1 vs comment:10000:ticket:1 - the latter in both comparisons the latter example should display as a missing link.

[Ryan J Ollos <ryano@…>]

Patch again r11086 of the trunk to address items (1) through (3).

[Ryan J Ollos <ryano@…>]

The second patch I'm posting addresses items (1) through (5). I plan to spend some more time with it tomorrow evening, but it is currently passing the unit tests, so I thought it would be worthwhile to post it for feedback.

[Ryan J Ollos <ryano@…>]

Patch again r11086 of the trunk to address items (1) through (5).

[Christian Boos]

Patch looks good, we'll have a look.

[Ryan J Ollos]

Replying to Ryan J Ollos <ryano@…>:

I added and modified some tests in trunk/trac/ticket/tests/wikisyntax.py​. As best I could determine though, without significant modifications to trunk/trac/wiki/tests/formatter.py​ it would not be possible to add a test for a comment:N link in wikisyntax.py since the tests are setup and executed in a wikipage. The comment here​ might be relevant, but I don't understand it.

It seems that TracLinks of the form comment:N can easily be tested by using the context parameter with formatter.suite.

[Ryan J Ollos]

Replying to Ryan J Ollos <ryano@…>:

1. comment:N and comment:N:ticket:M should display with class missing comment when ticket M does exist and the user has permission to view ticket M, but comment N does not exist. For example, compare: comment:1 vs comment:100000 and comment:1:ticket:1 vs comment:10000:ticket:1 - the latter in both comparisons the latter example should display as a missing link.

The comment class can't be used for TracLinks since it is already used on the ticket page for the div that encloses the comment text. I've added the class ticket-comment instead. I wonder though whether there is any value in adding this class, or whether I should just use the ticket class for TracLinks prefixed with ticket: or comment:.

I plan to give this another thorough review in the next day or so, but the latest proposed changes can be found in rjollos.git:t10742.

[Jun Omae]

LGTM except 416c4e86/rjollos.git. Many suite.addTest() lines in functionalSuite are commented out.

[Ryan J Ollos]

Thanks. Someday maybe I will investigate a way to allow running a single test in a module without resulting to commenting out all of the other tests. I'm bound to make this mistake again!

[Ryan J Ollos]

Unrelated change committed to 1.0-stable in [12117] and merged to trunk in [12118].

[Ryan J Ollos]

Replying to rjollos:

I've added the class ticket-comment instead. I wonder though whether there is any value in adding this class, or whether I should just use the ticket class for TracLinks prefixed with ticket: or comment:.

The latest proposed changes can be found in log:rjollos.git:t10742.2. The ticket-comment class has been removed and a test case has been fixed.

[Ryan J Ollos]

Committed with minor changes to 1.0-stable in [12126:12135] and merged to trunk in [12136]

[Ryan J Ollos]

In case anyone is able to cleanup the commit log messages, [12126] should be prefixed with 1.0.2dev, not 0.0.2dev.