Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web search display incorrect in bezel #39

Closed
cjs226 opened this issue Jan 1, 2010 · 28 comments
Closed

Web search display incorrect in bezel #39

cjs226 opened this issue Jan 1, 2010 · 28 comments
Labels

Comments

@cjs226
Copy link

@cjs226 cjs226 commented Jan 1, 2010

Web searches display incorrectly in first panel of Bezel interface. This was noticed in version B56a7 (3285) but persists in B57 (3840). This is a continuation of Bug 262 on the blacktree-quicksilver Issue Tracker which has a print screen:

Thanks, Clif

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 1, 2010

Confirmed.

Can you let me know what the icon was when it was previously working, and in what version it was last working? Which build (e.g. 3800)?

Are you on Leopard or Snow Leopard?

@cjs226
Copy link
Author

@cjs226 cjs226 commented Jan 1, 2010

You can see the icon in the 5th pic from the top in this howto:

I'm not sure, but will assume I was running B56a6 when it last worked.

I'm on Snow Leopard.

Thanks, Clif

@pkohut
Copy link

@pkohut pkohut commented Jan 2, 2010

@pkohut
Copy link

@pkohut pkohut commented Jan 2, 2010

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 2, 2010

Just looked, you're right - the square looks like the favicon from the site.

As this works with B54 on SL, somebody probably broke it with a commit they made between then and now sometime.

As you've said this is only a temporary fix, and as git is so good at git bisect and git blame, I suggest we try and find the original culprit that broke it and fix that.

Just incase someone else is looking:

good: B56a1
bad: B56a5
(needs narrowing down more)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 2, 2010

Don't some more bisecting:

034198f is the first bad commit
commit 034198f
Author: tiennou7 tiennou7@d44230c2-7321-0410-b0f2-3d6f2d243adf
Date: Wed Sep 17 10:09:07 2008 +0000

 r1002@myBook:  tiennou | 2008-08-08 13:47:13 +0200
 Memory leaks.
 We now enable NSZombieEnabled and NSDeallocateZombies when QSDebugMemory is set.


git-svn-id: https://blacktree-alchemy.googlecode.com/svn/branches/B5X@226 d44230c2-7321-0410-b0f2-3d6f2d243adf

:040000 040000 8e3e4c2d811e0af86c664693cd787b7fe4543b3e bb2c419d82b2eabd962f45a1746f775fd7ff4b8d M Quicksilver

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 2, 2010

Bug IS NOT
Quicksilver/PlugIns-Main/QSCorePlugIn/Code/QSFileTemplateManager.m
Quicksilver/Code-QuickStepInterface/QSSearchObjectView.m
Quicksilver/Code-QuickStepFoundation/NSBundle_BLTRExtensions.m

Still looking for the original culprit... :)

@cjs226
Copy link
Author

@cjs226 cjs226 commented Jan 2, 2010

Thanks pkohut!

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 3, 2010

Bug found to be lines 244-253 of QSResourceManager.m

  • NSString *type = [locator objectForKey:@"type"];

- NSString *basePath = [bundle bundlePath];

  • if (basePath)
  •  image = [workspace iconForFile:basePath];
    
  • else if (type)
  •  image = [workspace iconForFileType:type];
    
  •    if(bundle != nil) {
    
  •        image = [workspace iconForFile:[bundle bundlePath]];
    
  •    } else {
    
  •        if(DEBUG)
    
  •            NSLog(@"Unable to locate bundle with identifier %@, using locator %@", bundleID, locator);
    
  •        image = [workspace iconForFileType:[locator objectForKey:@"type"]];
    
  •   }
    

@pkohut, is this what you changed to fix the bug? Or did you edit somewhere else?

@pkohut
Copy link

@pkohut pkohut commented Jan 3, 2010

Looks like your on the right path.

In function imageNamed - variable image == nil after all tests had been done, so I added one last test in order to get a image pointer. What you've found is indirectly called by imageWithLocatorInformation which is called in imageNamed, which should provide the needed image pointer.

One other place I changed is in QSObject_URLHandling.m, function drawIconForObject. It's the actual code for drawing the icons in the panel and overlays the finder icon for query objects. In B57 it is also drawing the ugly mess for web search objects.

Here are the two files with the modifications
http://cloud.github.com/downloads/pkohut/blacktree-alchemy/Issue39.zip

@pkohut
Copy link

@pkohut pkohut commented Jan 5, 2010

Have you investigated this further Patrick? Go ahead and fix if you like, cause it will be a few days before I can get back on it.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 6, 2010

I haven't managed to do anything yet.

I've sure it's just those few lines of code I mentioned above, but I'm not sure what Etienne was trying to do with them when he changed it so I don't want to change it back then break something else.

I'm suddenly very busy, so it doesn't look like I'm gonna be able to do anything for a while now - Christmas is over, and it's back to work :(

@pkohut
Copy link

@pkohut pkohut commented Jan 7, 2010

I'll take care of it in the next day or two.

@pkohut
Copy link

@pkohut pkohut commented Jan 7, 2010

I went up one from commit 034198f, but it doesn't seem to be working either. Type GOO in QS and the web search icons are missing in the drop down (need the web services plugin). Also, there are no favicons shown.

Do you know of any good versions that will build, before commit 034198f? I downloaded 3 different ones and just my luck they're missing files and will not build.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 7, 2010

Hmm.... have you tried git checkout 034198f?

I've created a post in the dev group so as to make things nicer here for bug submitters:
http://groups.google.com/group/blacktree-alchemy-dev/

:)

@pkohut
Copy link

@pkohut pkohut commented Jan 7, 2010

Thanks Patrick, I've got a request pending to join the group. No I tried the one before 034198f and have made it all the way back to http://github.com/tiennou/blacktree-alchemy/commit/d1dcb491de7c77d75cc78125fc748e5ba22caa7e, and still the icons are missing from the drop down. Maybe someone else can compile from code to confirm what I'm seeing. TIA

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 7, 2010

Hmmm I didn't realise you weren't already a part of the group :(

I can confirm that the icons down't show up in the drop down menu, and the favicons don't show up in that build - but I was only looking at the point where the two icon problem arose.
(file emailed)

Here's what I said in the devs group (until you get access - could be a long time!)

The bug seems to have started around commit 034198f
To build anything before that you need the NSImage+QuickLook folder.
Paste the attached file in ROOT/Quicksilver/Code-External/

@pkohut
Copy link

@pkohut pkohut commented Jan 8, 2010

For those interested there is a new fix for this issue. Commit number a874418

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 9, 2010

Just built it and it looks pretty sexy ;)

Do you have any merge conflicts with that and the latest tiennou/blacktree-alchemy branch (I added a few commits recently). If not then I'll have a look at merging it and closing this :)

We really need to get another release out soon.... :D

@pkohut
Copy link

@pkohut pkohut commented Jan 9, 2010

Not sure about merge conflicts, but I am out of sync.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 9, 2010

:( Having just looked and closed bug #21, there still seems to be a few problems.

The bug reported that Firefox bookmarks were showing up with the ugly white file icon as opposed to the globe. Your fix removes the globe from behind it, but leaves the ugly white file icon :(

@pkohut
Copy link

@pkohut pkohut commented Jan 10, 2010

Did you add the QSResourceManager.m and .h changes?

@pkohut
Copy link

@pkohut pkohut commented Jan 11, 2010

Taken care of.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 11, 2010

Sorry, I hadn't done so.
Have you added them to the tiennou/B5X repository?

I've been busy lately so haven't had time to do any of this :(

@pkohut
Copy link

@pkohut pkohut commented Jan 11, 2010

I'll update in a little bit. The fix was simple, though any future items like these will need to be handled on a case by case basis. There are many areas where an object can have its image state set. I'll leave it at that for now. Later I post something in the dev group.

@pkohut
Copy link

@pkohut pkohut commented Jan 12, 2010

Latest commit updates for this issue are at e7b8ddd

@cjs226
Copy link
Author

@cjs226 cjs226 commented Jan 12, 2010

Thank you all VERY MUCH for the support! Is there anywhere the community can donate to the project?

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 28, 2010

Closed :)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants