Bug 177734 - [Patch] Search top-level domain for favicon.ico if bz domain doesn't have it
[Patch] Search top-level domain for favicon.ico if bz domain doesn't have it
Product: Fedora
Classification: Fedora
Component: eclipse-bugzilla (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Igor Foox
Depends On: 178077
  Show dependency treegraph
Reported: 2006-01-13 11:49 EST by Bug Testing
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-01-18 17:03:02 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
patch (I'll remove the unnecessary printlns) (3.83 KB, patch)
2006-01-17 11:25 EST, Andrew Overholt
no flags Details | Diff
Updated patch with Igor's suggestions and removed printlns. (3.69 KB, patch)
2006-01-17 13:35 EST, Andrew Overholt
no flags Details | Diff

  None (edit)
Description Bug Testing 2006-01-13 11:49:14 EST
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@redhat.com>

	* .cvsignore: Add bin/.
	* src/org/eclipse/team/bugzilla/operations/BugzillaOperations.java
	(getBugzillaIcon): Search main domain for favicon.ico if not found
	at bugzilla URL.
Comment 1 Andrew Overholt 2006-01-17 11:25:35 EST
Created attachment 123304 [details]
patch (I'll remove the unnecessary printlns)
Comment 2 Igor Foox 2006-01-17 11:45:00 EST
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 ...


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.
Comment 3 Andrew Overholt 2006-01-17 13:35:53 EST
Created attachment 123316 [details]
Updated patch with Igor's suggestions and removed printlns.

I'm committing this.
Comment 4 Igor Foox 2006-01-17 16:48:22 EST
As a result of the patch there's a problem with the Ubuntu bugzilla, adding
dependency on that bug.
Comment 5 Igor Foox 2006-01-18 17:03:02 EST
Patch commited.

Note You need to log in before you can comment on or make changes to this bug.