Vector: Remove slow collapsibleNav module
Closed, ResolvedPublic

Description

On translatewiki.net I see collapsibleTabs and collapsibleNav taking over 20 ms each on page load to do their work. Also on Wikipedia, but a bit less slow.


Version: unspecified
Severity: major
See Also:
T57905: user preference to override collapsing of sidebar items in Vector skin
T67444: Vector: Use a consistent indentation for all sidebar links

bzimport set Reference to bz39035.
Nikerabbit created this task.Via LegacyAug 4 2012, 11:18 AM
MZMcBride added a comment.Via ConduitDec 2 2013, 7:52 AM

Niklas: is this still an issue?

Nikerabbit added a comment.Via ConduitDec 2 2013, 8:37 AM

On one random page load at https://translatewiki.net/wiki/User:Nike

$.fn.collapsibleTabs took 102 ms
mw.uls.init took 87 ms
CollapsibleNav took 48 ms.

Nemo_bis added a comment.Via ConduitFeb 6 2014, 11:53 AM

From bug 51170:

(In reply to comment #7)

Does anybody know what makes collapsible navs so effing slow or run so late
on
beta.wmflabs? Can it even send an event when it is ready? Or is someone going
to remove it in a week?

(In reply to comment #8)

Likely because there's an effing lot of extensions installed, many of which
load and execute their own resources.

Core could send an event / fire a hook after collapsing the sidebar, if only
someone implemented it. It might be easier to fix this by doing less absolute
positioning in ULS instead (which would mean that the tooltip will be in the
correct position automagically).

(In reply to comment #9)

(In reply to comment #8)
> Likely because there's an effing lot of extensions installed, many of which
> load and execute their own resources.

Note that I was experiencing this bug on my own testwiki with almost no
extensions as well; this largely depends on your computer's speed (how much
time it takes it to render the page and execute other scripts, against the
hardcoded 500/700 ms limit).

gerritbot added a comment.Via ConduitMay 3 2014, 11:19 AM

Change 131259 had a related patch set uploaded by Nemo bis:
Remove collapsibleNav: performance cost too high

https://gerrit.wikimedia.org/r/131259

gerritbot added a comment.Via ConduitMay 4 2014, 1:03 AM

Change 131259 merged by jenkins-bot:
Remove collapsibleNav: performance cost too high

https://gerrit.wikimedia.org/r/131259

Nemo_bis added a comment.Via ConduitMay 4 2014, 5:50 AM

Niklas, how slow is collapsibleTabs now for you and what consequences does it have?

I wonder if that should be split to its own bug and this one be closed, as collapsibleTabs has lower effects: among other things, it should almost never be triggered for most users on a wiki by default (with English l10n on a Wikipedia as anon, it kicks in around 750 px of width, to hide "view history").

> this largely depends on your computer's speed (how much
> time it takes it to render the page and execute other scripts, against the
> hardcoded 500/700 ms limit).

(In reply to Gerrit Notification Bot from comment #5)

Change 131259 merged by jenkins-bot:
Remove collapsibleNav: performance cost too high

This was the main [[w:FOUC]] and slowness vector.

TheDJ added a comment.Via ConduitMay 4 2014, 9:10 AM

wow, so communities complained about this for 4 years, and now we just kill it within the span of a day ? Can I be surprised ?

I never really like the afterthought of how this was implemented. However, it is one of those areas that could easily have been improved probably. Moving the classes and some of the dom manipulation into the actual skin, removing the tabindex manipulation (which was pointless anyways) and probably using a less complicated delegate.

Krinkle added a comment.Via ConduitMay 4 2014, 1:38 PM

Please don't remove collapsibleTabs the same way. I think both of these components address valid UX issues.

If the UX testing data we gathered in the usability initiative has been of any value, the removal of collapsibleNav is going to be a notable UX regression. Especially the long language list on some pages, and the overall visual impact of them was quite big (they take up too much space by default, and attract too much attention, hiding them by default removed a lot of clutter).

However I agree, the way it was implemented as sufficiently badly performant that it's OK to remove for now, but we really should invest in re-implementing this ASAP.

However I don't think we should remove collapsibleTabs the same way. That one isn't too bad. It surely can be improved, but I think it is worth the minor perf hit compared to the value it adds.

Nemo_bis added a comment.Via ConduitMay 4 2014, 5:11 PM

(In reply to Krinkle from comment #8)

If the UX testing data we gathered in the usability initiative has been of
any value

(which we can't know because we've been waiting over 4 years for it to be published in anonymised form as promised back then), <continue sentence>

Nemo_bis added a comment.Via ConduitMay 17 2014, 9:07 AM

MatmaRex expected a lot of bugs would need to be closed, but I only found a handful (still a lot of things which are now better): https://bugzilla.wikimedia.org/buglist.cgi?list_id=314808&longdesc=bug%2039035&longdesc_type=substring&query_format=advanced
I skimmed about 400 bugs with very generic searches because keywords were unpredictable, but I may have missed something.

Aklapper added a comment.Via ConduitMay 17 2014, 12:52 PM

Thank you Nemo for going through these tickets!

Technical13 added a comment.Via ConduitMay 19 2014, 1:31 PM

So, this is why I no longer have the ability to collapse the sections of the sidebar I do not want to see. This has not improved my page load times at all (it has actually made them worse on my mobile device in desktop mode). This change is now also wasting valuable volunteer time having to scroll down through pages of unwanted links to get to the p-tb to get to the tools that are needed to work on pages (like reflinks and the anti-vandal tools). I have no issue with these sections not being collapsed by default if you think it will help some people with some historic version of some browser, but for everyone else, we should have the ability to collapse these sections and not have our workflows interrupted and broken.

Krenair added a comment.Via ConduitMay 21 2014, 3:20 PM

(In reply to Derk-Jan Hartman from comment #7)

removing the tabindex manipulation (which was pointless anyways)

#2014051610008141 suggests otherwise

He7d3r added a comment.Via ConduitMay 21 2014, 3:51 PM

(In reply to Alex Monk from comment #13)

(In reply to Derk-Jan Hartman from comment #7)
> removing the tabindex manipulation (which was pointless anyways)

#2014051610008141 suggests otherwise

What is this number?

Krenair added a comment.Via ConduitMay 21 2014, 3:54 PM

It's a ticket in the info-en::Tech issues OTRS queue

Krenair added a comment.Via ConduitMay 22 2014, 10:57 AM

Which I now have permission to share - is from a user who, due to pains in his right forearm, tries to use 'tab' to navigate where possible. The search box went from being the first tabindex (set by collapsibleNav) to being one of the last on the page.

Krenair added a comment.Via ConduitMay 22 2014, 11:01 AM

Gerrit change 134801

gerritbot added a comment.Via ConduitJul 25 2014, 4:44 PM

Change 145911 had a related patch set uploaded by Krinkle:
Vector: Add back collaspibleNav module

https://gerrit.wikimedia.org/r/145911

This was one of the worst changes Wikipedia has done and should have been rejected, even if it was slow.

He7d3r edited the task description. (Show Details)Via WebSun, Feb 22, 4:30 PM
He7d3r added a project: JavaScript.
He7d3r set Security to None.

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.