On certain bugs, show_bugs.cgi generates hundreds of consecutive lines of whitespace in the flags table. Some bugs seem worse than others - I believe due to what component they are in (and as such how many flags are shown) and also the state of those flags. I presume one of the [% FOR ... %] blocks has too many blank lines outside of the [% ... %] in the template. 1) View source of https://bugzilla.mozilla.org/show_bug.cgi?id=648866 2) Scroll down to <table id="flags"> Expected: As few blank lines of whitespace as possible (eg max one or two between each non-whitespace block). Actual: ~250 lines of whitespace, followed by a single flag <tr> block and then ~250 more lines of whitespace.

Confirmed. There are a few lines of whitespace using stock 4.0 on landfill.bugzilla.org but nowhere near the amount that BMO is adding. Looking into it. dkl

Basically the looping code in the flag list template inserts a lot of extra newlines since there are quite a few 'inactive' flags for the particular Product/Component combination. This patch will suppress the extra newlines so that the source looks more condensed when only a few flags from a large list are actually active. Please review dkl

Comment on attachment 531360 [details] [diff] [review] Patch to remove extra newlines in flag list html (v1) r=glob looks fine

Pushing upstream.

Comment on attachment 531360 [details] [diff] [review] Patch to remove extra newlines in flag list html (v1) I want to give it a look before approving this patch. It seems you are removing several newlines which will make the code harder to read. Readability is more important than some extra newlines.

(In reply to comment #5) > Readability is more important than some extra newlines. Yeah but 500 newlines?

(In reply to comment #6) > Yeah but 500 newlines? I'm not talking about the output, but the code itself.

Comment on attachment 531360 [details] [diff] [review] Patch to remove extra newlines in flag list html (v1) >+ [%- FOREACH type = flag_types -%] > [%# Step 1a: Display existing flag(s). %] >+ [%- FOREACH flag = type.flags -%] Are all these [%- -%] really needed? Do they really help removing newlines? >- [% END %] >- >+ [%- END -%] > [%# Step 1b: Display UI for setting flag. %] Removing the newline here makes the template really hard to parse, IMO. >- [% END %] >- >+ [%- END -%] > [%# Step 2: Display flag type again (if type is multiplicable). %] Same here. > [% END %] >- > [% PROCESS flag_row first_cell_empty = 0 addl_text = "addl." %] Same here. Please tell me if these removed newlines are really required. I have the feeling that they are not. In the case I'm wrong, please re-request review from me.

Here is a much more scaled down version of the patch. After doing some more experimentation, I was able to figure out where to collapse whitespace to bring down the line count significantly. And without any loss of readability on the template itself. dkl

Comment on attachment 542072 [details] [diff] [review] Patch to remove extra newlines in flag list html (v2) Yes, much better. Thanks! r=LpSolit

Thanks trunk: Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/trunk modified template/en/default/flag/list.html.tmpl Committed revision 7852. 4.0: Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/4.0 modified template/en/default/flag/list.html.tmpl Committed revision 7613. dkl