Description of problem: Currently we just search the BZ domain for favicon.ico. Often times, the BZ installation is on a subdomain with no favicon.ico (ex. bugs.eclipse.org). This patch attempts to get favicon.ico from the top-level domain before defaulting to no icon. I have also added bin/ to .cvsignore. Okay to apply? 2006-01-13 Andrew Overholt <overholt> * .cvsignore: Add bin/. * src/org/eclipse/team/bugzilla/operations/BugzillaOperations.java (getBugzillaIcon): Search main domain for favicon.ico if not found at bugzilla URL.
Created attachment 123304 [details] patch (I'll remove the unnecessary printlns)
Looks ok overall, a few comments. 1. You're doing: iconUrl = new URL(base.getProtocol() + "://" + newHost + "favicon.ico"); and then a few lines after: iconUrl = new URL("http://" + newHost + "/" + "favicon.ico"); I think you might need the extra "/" in the first case too. 2. It will be easier to read if you change the top level if statement from: if ((defaultIcon == null) || (defaultIcon.getImageData() == null)) { ... try top host ... } else { ... return non-null icon ... } to if ((defaultIcon != null) && (defaultIcon.getImageData() != null)) { return defaultIcon; } ... otherwise try to find in top host ... 3. In getBugzillaIconImageDescriptor() you do ImageDescriptor defaultIcon = ImageDescriptor.createFromURL(iconUrl); URLConnection con = iconUrl.openConnection(); int len = con.getContentLength(); but in the original code they first opened the connection and checked the length, and then got the full icon if not 0. Is there a reason you switched them? Not sure if it makes a big (or any) difference but might be better to check for 0 length first.
Created attachment 123316 [details] Updated patch with Igor's suggestions and removed printlns. I'm committing this.
As a result of the patch there's a problem with the Ubuntu bugzilla, adding dependency on that bug.
Patch commited.