Closed Bug 416007 Opened 12 years ago Closed 12 years ago
Bookmark details persist during search
(see attached image) If I select a bookmark, its details are visible in the details pane (bottom half of the screen) -- so far so good. If I then do a search, though, the search results begin filling in the upper half of the screen, leaving the details of the previously selected bookmark in the bottom half of the screen. This is problematic in that the details (a) are about a bookmark that has nothing to do with the search results and (b) occupy space that could be better used for the potentially long list of result-set bookmarks.
You want the bottom panel temporarily go away during a search?
same thing happens when looking at History. There is a bug filed on that. Bookmark properties should not be present unless a Bookmark is selected.
Ria: not exactly -- once a search is initiated (ie. when the user hits enter after typing in the search terms), the previously selected bookmark should no longer be selected. It has no relevance to the current task. Tracy: which bug is that?
(In reply to comment #3) > Ria: not exactly -- once a search is initiated (ie. when the user hits enter > after typing in the search terms), the previously selected bookmark should no > longer be selected. It has no relevance to the current task. Indeed. As soon as the search is run, the panel should revert to how it looks when a folder is selected ("n items; select an item to see it's properties") - to see what I mean, just select any folder like the "Unsorted Bookmarks" folder. > Tracy: which bug is that? Bug 407541
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
We can just clear the selection and be ok here, right?
Assignee: nobody → mak77
I think we can call the onContentTreeSelect method to display the bottom panel when searching. And I upload this proposal patch. Could anybody can help review it? Thanks!
Just call onContentTreeSelect in showSearchUI and doSearch method.
Comment on attachment 311563 [details] [diff] [review] A proposal patch Request review
Bo, imo the correct way of dealing with this would be reset the selection on the content tree on search
(In reply to comment #9) > Bo, imo the correct way of dealing with this would be reset the selection on > the content tree on search > Do you mean in doSearch and search, call clearSelection() of the content tree to make the onselect handler to display the panel? Just like the new patch? Thanks for your advice!
Clear selection to make onselect handler to deal with the panel displaying.
change comments //Display the panel to something like // Clear right pane selection to update details panel then ask review to Mano
Assignee: mak77 → Techrazy.Yang
(In reply to comment #12) > change comments > > //Display the panel > > to something like > > // Clear right pane selection to update details panel > > then ask review to Mano > Thank you very much!
Make the comment more readable. Thanks to Macro(mak77) very much!
Comment on attachment 311953 [details] [diff] [review] Change the comment I would rather just call onContentTreeSelect directly, the old view is already gone at this point, thus clearing the selection doesn't make sense.
Attachment #311953 - Flags: review?(mano) → review-
(In reply to comment #15) > (From update of attachment 311953 [details] [diff] [review]) > I would rather just call onContentTreeSelect directly, the old view is already > gone at this point, thus clearing the selection doesn't make sense. > Do you mean this old patch is ok? https://bugzilla.mozilla.org/attachment.cgi?id=311563
Comment on attachment 311563 [details] [diff] [review] A proposal patch Yes the first patch, this is my fault, sorry Bo :( + + //Update the bottom panel + PlacesOrganizer.onContentTreeSelect(); fix trailing space in the first empty line ad space after // in comments and use details pane instead of bottom panel. Then ask for Mano review
Attachment #311563 - Attachment is obsolete: false
Whiteboard: [has patch][needs review mano] → [has patch][has review]
Change back to onContentTreeSelect function call.
Comment on attachment 313039 [details] [diff] [review] Revised patch r=mano
Attachment #313039 - Flags: review?(mano) → review+
Checking in browser/components/places/content/places.js; /cvsroot/mozilla/browser/components/places/content/places.js,v <-- places.js new revision: 1.151; previous revision: 1.150 done
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040904 Minefield/3.0pre Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre
Status: RESOLVED → VERIFIED
Test case 6295 has been updated to reflect this bug for future regression testing.
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.