"Priority Breakdown" report uses no_break filter, resulting in error

RESOLVED FIXED in 3.0

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: f1sh, Assigned: f1sh)

Tracking

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
Since 'no_break' was removed in Bugzilla 5.0+, Priority Breakdown reports result in a error as it looks for the missing filter. This report needs to be modified so that it uses the 'nowrap' class, as changed in bug 880282
(Assignee)

Comment 1

3 years ago
Created attachment 8693785 [details] [diff] [review]
patch.diff

Easy patch. Tested and works as expected.
Assignee: gregaryh → theycallmefish
Status: NEW → ASSIGNED
Attachment #8693785 - Flags: review?(LpSolit)
(Assignee)

Updated

3 years ago
Target Milestone: --- → 3.0

Comment 2

3 years ago
Comment on attachment 8693785 [details] [diff] [review]
patch.diff

>           [% FOREACH p = priorities.keys.sort %]
>           <tr>
>-            <td style="border-right: 2px solid black; font-weight: bold; text-align: left;">[% p FILTER no_break %]</td>
>+            <td class="nowrap" style="border-right: 2px solid black; font-weight: bold; text-align: left;">[% p FILTER none %]</td>
>           </tr>
>           [% END %]
>         </table>

The patch is broken:

  patch: **** malformed patch at line 14: @@ -123,7 +123,7 @@


In my copy of this template, there are 9 <td> in this <tr> while there is only one in your patch.
Attachment #8693785 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 3

3 years ago
Created attachment 8693802 [details] [diff] [review]
patch-v2.diff

My mistake. The previous patch had pieces of the Testopia consolidation in it that I incorrectly attempted to remove. Here is a new patch.
Attachment #8693785 - Attachment is obsolete: true
Attachment #8693802 - Flags: review?(LpSolit)

Comment 4

3 years ago
So which patch should be applied first? The other one first, right?
(Assignee)

Comment 5

3 years ago
(In reply to Frédéric Buclin from comment #4)
> So which patch should be applied first? The other one first, right?

I reverted the file to trunk, so this one can be approved first.
(Assignee)

Comment 6

3 years ago
On second thought, the other one would probably be best first, so that this one fixes that one filter area instead of the patch reverting.
(Assignee)

Comment 7

3 years ago
Otherwise, I will need to write a new patch for tr_list_caseruns.

Comment 8

3 years ago
Err, I'm a bit confused. How is this patch related to this bug?? Isn't this patch about bug 1229135 instead?
(Assignee)

Comment 9

3 years ago
This patch fixes the issue with priority breakdown reports resulting in an error. Ultimately, the problem in bug 1229135 is present on clickable links generated from this report, but that is a list problem rather than a report problem.

Comment 10

3 years ago
Comment on attachment 8693802 [details] [diff] [review]
patch-v2.diff

>-            <td style="border-right: 2px solid black; font-weight: bold; text-align: left;">[% p FILTER no_break %]</td>
>+            <td class="nowrap" style="border-right: 2px solid black; font-weight: bold; text-align: left;">[% p FILTER none %]</td>

We are in a HTML page, so it should be replaced by |FILTER html| instead, to prevent code injection. And the patch no longer applies cleanly due to the other checkin. Could you update it, please? :)
(Assignee)

Comment 11

3 years ago
Created attachment 8694362 [details] [diff] [review]
patch-v3.diff

Switched FILTER to html and wrote patch against current trunk
Attachment #8693802 - Attachment is obsolete: true
Attachment #8693802 - Flags: review?(LpSolit)
Attachment #8694362 - Flags: review?(LpSolit)

Comment 12

3 years ago
Comment on attachment 8694362 [details] [diff] [review]
patch-v3.diff

Thanks! :) r=LpSolit
Attachment #8694362 - Flags: review?(LpSolit) → review+

Comment 13

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/extensions/Testopia.git
   8703d68..97b81ca  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.