launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Henning Eggers has proposed merging lp:~henninge/launchpad/bug-813540-dupefinder into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #813540 in Launchpad itself: "Bug icons in dupefinder listing get pushed to next line"
  https://bugs.launchpad.net/launchpad/+bug/813540

For more details, see:
https://code.launchpad.net/~henninge/launchpad/bug-813540-dupefinder/+merge/68557

= Summary =

Two reasons for bug 813540 both related to browsers reflowing the
of the content of the table cell that the bug icon is in.

- Labels for bugs without relevant bugtasks contained whitespace which
  caused the browser to insert a new line because the cell seems to be
  too narrow.
- If the bug title is very long, this happens even for bugs with
  relevant bugtask (other code path in template) because of white space
  between tags.

== Proposed fix ==

The real fix would be to replace the <table> layout with something else
and allow long bug titles to be wrapped. I did not want to start on that
as part of this critical bug.

I simply removed all the whitepsace in and around the affected tags.

== Pre-implementation notes ==

Talked to danilo.

== Demo and Q/A ==

Create a bug with a looooooong title, then use the dupfinder to find
and display it.
I have not yet found out how to create a "relevant" bugs but a
closer look at the code in getRelevantBugTask in the view could
help with that.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/templates/bugtarget-macros-filebug.pt
-- 
https://code.launchpad.net/~henninge/launchpad/bug-813540-dupefinder/+merge/68557
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/bug-813540-dupefinder into lp:launchpad.

=== modified file 'lib/lp/bugs/templates/bugtarget-macros-filebug.pt'
--- lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2011-07-18 14:43:04 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2011-07-20 15:16:34 +0000
@@ -349,15 +349,14 @@
             <td class="bug-already-reported-expander">
               <tal:comment condition="nothing">
                 We use zero-width space (&#8203;) to force the otherwise
-                empty span using sprite image to show up.
+                empty span using sprite image to show up. Also no whitespace
+                between tags is allowed.
               </tal:comment>
-              <span class="expander">&#8203;</span>
-              <label tal:attributes="for string:bug-already-reported-as-${bug/id};
+              <span class="expander">&#8203;</span><label
+                 tal:attributes="for string:bug-already-reported-as-${bug/id};
                                      class bugtask/image:sprite_css"
-                     tal:condition="bugtask">&#8203;</label>
-              <label class="sprite bug" tal:condition="not:bugtask">
-                &#8203;
-              </label>
+                     tal:condition="bugtask">&#8203;</label><label
+                 class="sprite bug" tal:condition="not:bugtask">&#8203;</label>
             </td>
             <td>
               <div>