Bug 6616 - Double-clicking on a search result seems broken
Summary: Double-clicking on a search result seems broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-17 12:24 PST by Geoffrey Garen
Modified: 2006-03-04 15:05 PST (History)
0 users

See Also:


Attachments
Patch (40.86 KB, patch)
2006-03-03 14:31 PST, Timothy Hatcher
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2006-01-17 12:24:52 PST
To Reproduce:
1. Navigate to www.google.com
2. Open inspector.
3. Search for "google"
4. Double click a search result

You always seem to end up with the <body> element selected in the tree view. I would expect the the tree 
view to expand and reveal the double-clicked element.
Comment 1 Timothy Hatcher 2006-03-03 14:31:30 PST
Created attachment 6830 [details]
Patch

This patch mainly fixes this bug, however it has cleanup and fixes for other minor bugs.

<rdar://problem/4411822> wrong element shown in Inspector inspecting main image at apple.com
<rdar://problem/4411908> in the Web Inspector, state of disclosure triangles should be preserved after search
http://bugzilla.opendarwin.org/show_bug.cgi?id=6616
Bug 6616: Double-clicking on a search result seems broken
http://bugzilla.opendarwin.org/show_bug.cgi?id=6709
Bug 6709: TypeError: Value undefined (result of expression treeScrollbar.refresh) is not object.

Code clean up and move more code into JavaScript.
Removes a few unused ObjC methods.
Many search fixes. Reveals the focused node when exiting the search.
Shows a "No Selection" screen when there are no search results.
Shows a node count for the number of results.
Fixes a couple of TypeErrors that show on the console.
Uses the system selection color in the Style pane tables.
Comment 2 Darin Adler 2006-03-04 11:56:39 PST
Comment on attachment 6830 [details]
Patch

The finalize method needs to call [super finalize]. But also, as long as there's an observer installed, the object will never get finalized because the pointers in the notification center will keep it alive. You need to come up with some other way to handle the object lifetime. Some way of "shutting down" other than actually deallocating the object that will work both with and without GC. If this is a global object that's never deallocated, then you just can not worry about it.

setSearchQuery: and searchPerformed: should use copy on the passed-in string instead of retain.

+    BOOL selectFirst = (![_private->searchResults count] ? YES : NO);

No need for ? YES : NO on something that's already a boolean.

I noticed that for JavaScript you aren't putting the start brace for functions on a separate line. I think you should match our C coding style more closely there.

+    } else if(!query.length && searchActive) {

Above line is missing a space after the if.

+        <input id="search" type="search" autosave="nodeSearch" results="20" placeholder="Search" incremental="incremental" onsearch="performSearch(this.value)" />

Not sure why you are using self-closing tags in this file. Is it XHTML or HTML?

Generally looks good. I'm going to mark it review+ even though the finalize method is wrong.
Comment 3 Timothy Hatcher 2006-03-04 15:05:35 PST
Thanks for pointing out the finalize method mistake. I have changed the notification center registration and de-registration to happen on window open and close.

I have changed the function declarations in JavaScript to match our style guidelines.

The file is intended to be XHTML. This was done before I knew how we handled xhtml as html. We should name the file with .xhtml then.

Committed in r13128. ( And r13131 since I forgot to save 2 files before committing.)