Bug 177734
| Summary: | [Patch] Search top-level domain for favicon.ico if bz domain doesn't have it | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Bug Testing <bugtesting> | ||||||
| Component: | eclipse-bugzilla | Assignee: | Igor Foox <ifoox> | ||||||
| Status: | CLOSED NEXTRELEASE | QA Contact: | |||||||
| Severity: | medium | Docs Contact: | |||||||
| Priority: | medium | ||||||||
| Version: | rawhide | CC: | overholt | ||||||
| Target Milestone: | --- | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2006-01-18 22:03:02 UTC | Type: | --- | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | 178077 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Bug Testing
2006-01-13 16:49:14 UTC
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. |