Closed Bug 392497 Opened 15 years ago Closed 14 years ago

search in history sidebar loses sort

Categories

(Firefox :: Bookmarks & History, defect)

3.0 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: tuukka.tolvanen, Assigned: tbertels+bugzilla)

References

Details

(Keywords: regression, verified1.9.1)

Attachments

(2 files, 2 obsolete files)

trunk firefox 20070812, linux. me being lazy, here's a copy of bug 364325 comment 0:

Steps to reproduce:

- start a new profile with blank history
- open the sidebar, set "View" to "By Last Visited"
- visit https://bugzilla.mozilla.org/show_bug.cgi?id=257334
- visit https://bugzilla.mozilla.org/show_bug.cgi?id=362003
- verify that bug 362003 is on top in the history sidebar (that's correct)
- now do a history search for "history.dat" in the sidebar

Result: both entries change position: the last visited address is no longer on
top (like in Firefox 2.0)
Component: History → Places
QA Contact: history → places
I don't see this with 3.0b5pre 2008031504, although I see the reverse at #423296 .
My bad, I misunderstood, the bug is still there.
This keeps the sort order for the lastvisited and visited sort orders.
Attachment #309991 - Flags: review?(mano)
This is a regression from Firefox 2.
Keywords: regression
Flags: wanted1.9.0.x?
Mano, do you have time to review this?
Assignee: nobody → tbertels+bugzilla
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: wanted-firefox3.1?
nice to have, not an issue we're going to prioritize for 3.1
Flags: wanted-firefox3.1? → wanted-firefox3.1-
Why is this a separate bug ?
This is a bug on all platforms. Even if bug 364325 is currently marked as fixed. It is not.
See my comment there.
(In reply to comment #7)
I don't see which comment you're talking about, but yes it's reproduceable everywhere.
OS: Linux → All
Hardware: PC → All
Version: Trunk → 3.0 Branch
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 364325
Sorry for bugspam, the culprit seems to be actually bug 389782.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Duplicate of this bug: 434284
Duplicate of this bug: 452239
Attachment #309991 - Flags: review?(mano) → review+
Comment on attachment 309991 [details] [diff] [review]
Keep sort order when searching in sidebar history

r=mano code wise, please seek for ui-review.
Attachment #309991 - Flags: ui-review?(mconnor)
Attachment #309991 - Flags: ui-review?(mconnor) → ui-review?(faaborg)
I'm a little confused on this, wouldn't relevance be the most important sort order when searching? (or is our search really just more of a filter at the moment?)
Attachment #309991 - Flags: ui-review?(faaborg) → ui-review+
Comment on attachment 309991 [details] [diff] [review]
Keep sort order when searching in sidebar history

We can disgrard my previous comment, ranking and sort are two different things, so if we eventually rank based on
frecency (like the location bar does) that is an independent decision from
maintaining sort, which we should do either way.  (The exception being "most
visited" and "last visited" which would of course still effect ranking.)
Attached patch unbitrotSplinter Review
Attachment #309991 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/14997a124841
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Comment on attachment 358378 [details] [diff] [review]
unbitrot

asking approval, low risk
Attachment #358378 - Flags: approval1.9.1?
Duplicate of this bug: 483215
Can we get a test for this, please?
Attached patch browser-chrome test (obsolete) — Splinter Review
yes, we can!
Attachment #369407 - Flags: review?(adw)
Flags: in-testsuite?
Comment on attachment 369407 [details] [diff] [review]
browser-chrome test

Only some nits:

>+var pages = [
>+  "http://sidebar.mozilla.org/a",
>+  "http://sidebar.mozilla.org/b",
>+  "http://sidebar.mozilla.org/c",
>+  "http://www.mozilla.org/d",
>+];

A comment here explaining that these are listed by descending visit date would be useful.

>+const FILTERED_COUNT = 1;

A comment about how this is used.

>+      sidebar.contentDocument.getElementById("bylastvisited").doCommand();

A comment here like, "sort by visit date descending."

>+      //sidebar.contentDocument.contentWindow.searchHistory(this.value);

Did you mean to leave this in?

>+  is(rc, aExpectedRows, "Rows found: " + rc);
>+  var columns = tree.columns;
>+  is(columns.count, 1, "Columns found: " + columns.count);
>+  for (var r = 0; r < rc; r++) {
>+    var node = treeView.nodeForTreeIndex(r);
>+    is(node.uri, pages[r], "node is correct: " + node.uri);
>+  }

I think the strings in is() should be a description of what you expect, like, "there should be only one column in tree", "position of node in tree should be the same as the order it was visited in".  On failure the harness prints out the first argument to is() anyway right?

r=adw
Attachment #369407 - Flags: review?(adw) → review+
Attached patch test for checkinSplinter Review
fixed Drew's comments
Attachment #369407 - Attachment is obsolete: true
test pushed
http://hg.mozilla.org/mozilla-central/rev/ead10c322220
Flags: in-testsuite? → in-testsuite+
Duplicate of this bug: 486341
Comment on attachment 358378 [details] [diff] [review]
unbitrot

a191=beltzner, please make sure the test lands with it
Attachment #358378 - Flags: approval1.9.1? → approval1.9.1+
verified FIXED on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090608 Minefield/3.6a1pre ID:20090608122028

as well as Fx3.5RC1 on win XP
Status: RESOLVED → VERIFIED
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.