30694 - dependency reporting and unconfirmed bugs (strikethrough, crossed out)

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There are two problems with dependency reporting and unconfirmed bugs, probably
both one line fixes:

1) In the "Bug 25258 depends on:" section in
http://bugzilla.mozilla.org/show_bug.cgi?id=25258 , bug 30671 is shown in
strikeout text, but it's not resolved, verified, or closed.

2) In the email reports, there's a "[]" that shouldn't show:


http://bugzilla.mozilla.org/show_bug.cgi?id=25258

*** shadow/25258        Fri Mar  3 17:45:33 2000
--- shadow/25258.tmp.29879      Mon Mar  6 06:42:12 2000
***************
*** 15,20 ****
--- 15,21 ----
  URL: 
  Cc: [e-mail], [e-mail], [e-mail],
[e-mail]
  Summary: Selected FORM fields and links do not redraw when losing focus
+ BugsThisDependsOn: 30671[]
  OtherBugsDependingOnThis: 18579
  
  When I select a field inside any form, whether it be with the mouse or via
TAB,

Comment 1

[Terry Weissman]

[e-mail] is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Comment 2

[Jesse Ruderman]

*** Bug 40895 has been marked as a duplicate of this bug. ***

Comment 3

[Jacob Steenhagen]

Patch forthcoming.  What this does is makes UNCONFIRMED bugs in the dependency
list italics instead of strick through.  Also, I think I found the stray [] in
the old mail system and patched that as well.

FYI, this was a seven line patch (I could have made it 3, but wanted to maintain
the same style that was already in use).

Comment 4

[Jacob Steenhagen]

Attachment: [PATCH] Make unconfirmed bug Italics instead of strike through in dependancy list

Comment 5

[Jacob Steenhagen]

Attachment: Oops... correct STRIKE and Italics problem

Comment 6

[Jacob Steenhagen]

Left out the logic in my patch to make UNCONFIRMED Italics instead of Strike
(the fist patch will make it both).

FYI I don't have a working copy of bugzilla (I hope to get one within the next
month or two) so this code hasn't been tested, only studied.

Comment 7

[Jacob Steenhagen]

OK, I finally have a working bugzilla installation to hack on (on my laptop of
all places...)

Anyway, I've got one last patch to submit.  This one has been tested and works
as advertised.

Comment 8

[Jacob Steenhagen]

Attachment: [PATCH] A tested patch that makes UNCONFIRMED appears as italics in dependency list

Comment 9

[Jacob Steenhagen]

Attachment: [ALT PATCH] Adds a subroutine for linking bugs

Comment 10

[Jacob Steenhagen]

The new "ALT PATCH" I just attached is a "bulk fix" for this issue and the one 
mentioned "post fix" for bug 59515.  It creates a new subroutine that 
formulates the <a> tag for a bug link.

Downside: Initiates another SQL call for each dependency

Upside: Provides a standard interface for linking bugs.  Can be called from the 
dependancy list and long description (as this patch does) as well as anywhere 
else a bug needs to be linked.  It will then provided a standard usage of the 
title attibute as well as the <strike> and <i> for resolution.

Future Upside:  This also makes it possible to add a session cache of this 
information so if the same bug is referenced more than once in a bug report it 
only needs to be sent to the SQL server once.  There's more on this in the 
comments in the patch.

Comment 11

[Jacob Steenhagen]

I'd really like to see this patch make it into 2.12.  My preference (of course) 
is the ALT PATCH, but if that's a bit too "agressive" the other one works.

Comment 12

[Jacob Steenhagen]

If I'm not mistaken, the SQL Call for dependencies can be changed from:
SendSQL("select dependencies.$targetfield from dependencies, bugs
  where dependencies.$myfield = $id and bugs.bug_id = dependencies.$targetfield
  order by dependencies.$targetfield");

To:
SendSQLSendSQL("select $targetfield from dependencies where  
  $myfield = $id order by $targetfield");

Because the Status isn't being pulled anymore (from the bugs table).

Comment 13

[Tara Hernandez]

Still looking at this bug, but want to hold off until after dmose lands his
email patch as there's overlapping code.  Putting it as dependant to make sure
this is seen.

Comment 14

[Jacob Steenhagen]

Attachment: Patch v2 - Update to current CVS

Comment 15

[Dave Miller [:justdave]]

r= [e-mail]

had to run the patch by hand for some strange reason...  the new patch I created 
was almost identical, so no clue why it wouldn't run the first time.

This is checked in.

Comment 16

[Jacob Steenhagen]

Other than the SQL statement not being changed (as mentioned in a comment and
included in the last patch), it's looks like a good checkin to me...


SendSQL("select $targetfield from dependencies where  
         $myfield = $id order by $targetfield");

Comment 17

[Jacob Steenhagen]

Reopening so I can get a diff for the SQL change

Comment 18

[Jacob Steenhagen]

Also, I forgot to include the 1 line change in processmail (for old-mail) as
mentioned in the original bug report.

Comment 19

[Jacob Steenhagen]

Attachment: Portion of fix let out from checkin

Comment 20

[Dave Miller [:justdave]]

re-fixed.  The additional patch is checked in.

Comment 21

[Stephan Niemz]

IMHO the <strike> tag should be avoided wherever possible.
For one, it is deprecated - see
http://www.w3.org/TR/1999/REC-html401-19991224/present/graphics.html#h-15.2.1

But more important is that it makes the text very unreadable, and it looks
ugly.  See bug 40286 for an example.

IMHO the Right Way to do it is to change the text (and/or background) color,
ideally with CSS.  Unfortunately this is already checked in, although not yet
active at mozilla.org.  So maybe I have to file a new bug. :-(

Comment 22

[Tara Hernandez]

Sorry for the spam, but I needed to be able to query for all of these correctly.

Comment 23

[Gervase Markham [:gerv]]

Can't verify - no unconfirmed bugs on Landfill :-(

Gerv

Comment 24

[Dawn Endico]

this works although having it be italicised is a pretty subdle difference.
its easy to overlook the fact that there's something different. I'd rather see
this be red.

Comment 25

[Dawn Endico]

users can view the summary of any bug despite group permissions. If i log
out and view bug 22041, the tooltip for the link to bug 17494 in dave's
3/11 comment shows me the summary even tho i am not permitted to view the
bug itself. 

Comment 26

[Dawn Endico]

jake suggests that a quick fix for this would be to omit the
title= field if the bug has a groupset other than 0.

Comment 27

[Jacob Steenhagen]

Attachment: Bugfix Patch v1

Comment 28

[Jacob Steenhagen]

This is the quick and dirty fix I mentioned in IRC.  This simply disables
showing of the summary (in either tooltips or the page title) of any bug that
has a groupset other than 0.

Comment 29

[timeless]

looks good for 2.12, should we aim for a better fix for 2.16?

Comment 30

[Jacob Steenhagen]

I think we should aim for a better fix in 2.14 (this is, after all, a security
issue).

Comment 31

[Jacob Steenhagen]

Attachment: untested patch for duplicates.cgi

Comment 32

[Jacob Steenhagen]

The patch I just attached is to fix the same security hole in duplicates.cgi
(leaking the summary).  It is as of yet untested as I haven't sat down to figure
out how duplicates.cgi works.  I should be able to look at that tonight and test
this patch.

Comment 33

[Jacob Steenhagen]

Dawn checked in the "quick fix".  Bug 71767 is about doing this properly for the
next release.

Comment 34

[Dave Miller [:justdave]]

Moving closed bugs to Bugzilla product