1562489 - Sorting by Location in Library is broken in some cases

Status: VERIFIED FIXED

(Toolkit :: Places, defect, P3)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

Attachment: bug.png

STR:

1. Start Firefox Portable with clean new fresh profile without any addons (extensions, plugins, themes, etc.)
2. Restore bookmarks backup from "bookmarks-test.json" file
3. Open Library (Ctrl+Shift+B keyboard shortcut)
4. Go to Bookmarks Toolbar
5. Press "Location" sorting button
6. Scroll to bottom
   and see that something is not right

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

Attachment: bookmarks-test.json

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:Virtual, if you think that's a regression, then could you try to find a regression range in using for example mozregression?

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

Oddly when I'm using sorting by "Location" (reversed), everything is sorted properly.

Comment 4

[Marco Bonardo [:mak]]

I can reproduce, the comparator in nsNavHistoryResult seems to do the right thing, thus I wonder if it's a bug of the treeview

Comment 5

[Marco Bonardo [:mak]]

I remember there was a request to Sort By Name sort groups between separators, see bug 262329. Though, that follows a different code path, and should not involve folders. It should be unrelated because this is a temporary sorting in the view.

I think the problem here is just that location-less items should be put at the beginning (in ascending order), that's what most "explorers" do anyway. What we do today is just to fallback to sorting by name, that confuses the sorting result.
It should be an easy fix.

Comment 6

[Marco Bonardo [:mak]]

Attachment: Bug 1562489 - Sorting bookmarks by Location should put containers first. r=Standard8

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/255f27d1127e
Sorting bookmarks by Location should put containers first. r=Standard8

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/255f27d1127e

Comment 9

[Ryan VanderMeulen (PTO, back July 27)]

AFAICT this is an old issue which we probably don't need to worry about backporting, but feel free to nominate the patch for Beta approval if you feel strongly otherwise.

Comment 10

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 70.0a1 (2019-07-11), so I'm marking this bug as VERIFIED.
Thank you very much! \o/