Bug 6616

Summary:

Double-clicking on a search result seems broken

Product:

WebKit

Reporter:

Geoffrey Garen <ggaren>

Component:

Web Inspector (Deprecated)

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
Patch	darin: review+

Geoffrey Garen

Reported 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.

Attachments
Patch (40.86 KB, patch) 2006-03-03 14:31 PST, Timothy Hatcher	darin: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Timothy Hatcher

Comment 1 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.

Darin Adler

Comment 2 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.

Timothy Hatcher

Comment 3 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.)

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