[Tags] Incorrect sub-tags ordering in Chrome

[tobyzerner]

From @tobscure on August 28, 2015 2:56

From @tarunmarkose on August 20, 2015 7:37

The "Installation" subtag of "Support" on the demo site opens wrongly under "Dev". This happens for me while using Chrome. Firefox and Safari, are fine.

Copied from original issue: flarum/core#239

Copied from original issue: flarum/tags#17

[tobyzerner]

From @tarunmarkose on August 27, 2015 8:26

Hey, @tobscure if you are not fixing this before the beta release, I and my colleague @ishanAhuja could try and take care of it. Let me know.

[tobyzerner]

Go for it! Relevant file is js/lib/utils/sortTags in the flarum/tags repo.

[ishanAhuja]

I'm unable to reproduce this issue. Has it been fixed?

[tobyzerner]

No, I can still reproduce in Chrome 44.

[tarunmarkose]

@tobscure Seems like something fixed the issue.. if you can still reproduce the error, please outline here..

[tarunmarkose]

I am on Chrome 44.0.2403.157.

[tobyzerner]

And seems to be fixed for me now too! That's weird, but sure, I'll take it :)

[tobyzerner]

I think there might be more to the issue – like the number of other tags are present. We need to come up with a solid reproduction method.

[tarunmarkose]

True dat, @maxyc's issue seems like the same problem. Even and odd number of tags is the first thing that comes to mind. Alright, lets figure this out! I and @ishanAhuja will first work on reproducing the error today.

@maxyc Is there anything you can add to this thread to help us reproduce the error? Can you take a screenshot of your tags administration page and put it up here so we can recreate it at our end?

[tarunmarkose]

@maxyc An export of your tags table, taken when the problem occurs would also be helpful.

[maxyc]

ok, on mobile error too

[maxyc]

error here http://novour.ru/t/Hobby

[maxyc]

http://novour.ru/sxd/backup/maxyc_novour_2015-08-31_10-46-44.sql.gz db table without users table

[tarunmarkose]

Thank you! I was able to recreate the issue. Will report back soon.

-Tarun

On Mon, Aug 31, 2015 at 1:17 PM, Максим notifications@github.com wrote:

http://novour.ru/sxd/backup/maxyc_novour_2015-08-31_10-46-44.sql.gz db table without users table

— Reply to this email directly or view it on GitHub https://github.com/flarum/core/issues/325#issuecomment-136290112.

[maxyc]

bug in Firefox 40.0 / Windows 7 http://api.browsershots.org/png/original/53/53432502fe92f0c63372bab91a76ade9.png

safari and opera is ok http://api.browsershots.org/png/original/49/495195c8ad1887ba048948d0c6542b27.png http://api.browsershots.org/png/original/1f/1fe9fe988c9e3596b44271fb5dad0b73.png

[tobyzerner]

Since I added a new sub-tag, this can now be reliably reproduced on discuss.flarum.org now.

[ishanAhuja]

It seems that sortTags(tags) (js/lib/utils/sortTags) is giving differently ordered arrays in Chrome and Safari. Consequently the tags are ordered differently on Chrome and Safari. Any insights on what may be the cause?

[tobyzerner]

It's due to a flaw in our sorting algorithm, so it ends up relying on the browser's sorting algorithm which varies between browsers. We need to fix our sorting algorithm.

[ishanAhuja]

I see. I'll take a stab at redoing the sortTags function. If you could provide a sample set of tags passed to sortTags and the expected output (including orphan tags), it would be very helpful. Thanks.

[tobyzerner]

The algorithm should basically result in this:

1. Tags where position !== null, ordered by position ascending.
   • Tags with a parent should immediately follow it. Tags with the same parent should be ordered by position ascending.
2. Tags where position === null, ordered by discussionsCount descending.

So given the tags:

id	name	position	parent_id	discussions_count
1	other	4	null	10
2	ux	2	null	13
3	dev	3	null	14
4	blog	1	null	1
5	i18n	2	3	7
6	extensibility	0	3	3
7	theming	1	3	1
8	likes	null	null	7
9	tags	null	null	19
10	sticky	null	null	2

We should end up with:

1. blog
2. ux
3. dev
4. extensibility
5. theming
6. i18n
7. other
8. tags
9. likes
10. sticky

The current implementation of the algorithm is almost there, there's just something slightly wrong with it.

[ishanAhuja]

Thanks Toby, I'll get on it.

On 04-Sep-2015, at 5:59 pm, Toby Zerner notifications@github.com wrote:

The algorithm should basically result in this:

Tags where position !== null, ordered by position ascending. Tags with a parent should immediately follow it. Tags with the same parent should be ordered by position ascending. Tags where position === null, ordered by discussionsCount descending. So given the tags:

id name position parent_id discussions_count 1 other 4 null 10 2 ux 2 null 13 3 dev 3 null 14 4 blog 1 null 1 5 i18n 2 3 7 6 extensibility 0 3 3 7 theming 1 3 1 8 likes null null 7 9 tags null null 19 10 sticky null null 2 We should end up with:

blog ux dev extensibility theming i18n other tags likes sticky — Reply to this email directly or view it on GitHub.

[ishanAhuja]

I've been pretty caught up at work, with a new release coming up for our product. If anyone else is willing to take a shot at this and contribute a fix, please go right ahead.

On 04-Sep-2015, at 6:28 PM, Ishan Ahuja ishanahuja@gmail.com wrote:

Thanks Toby, I'll get on it.

On 04-Sep-2015, at 5:59 pm, Toby Zerner notifications@github.com wrote:

The algorithm should basically result in this:

Tags where position !== null, ordered by position ascending. Tags with a parent should immediately follow it. Tags with the same parent should be ordered by position ascending. Tags where position === null, ordered by discussionsCount descending. So given the tags:

id name position parent_id discussions_count 1 other 4 null 10 2 ux 2 null 13 3 dev 3 null 14 4 blog 1 null 1 5 i18n 2 3 7 6 extensibility 0 3 3 7 theming 1 3 1 8 likes null null 7 9 tags null null 19 10 sticky null null 2 We should end up with:

blog ux dev extensibility theming i18n other tags likes sticky — Reply to this email directly or view it on GitHub.