Well, when you disable/enable a tool, there's a bunch of styles that stop/start applying. We can't really work around this. I'm assuming that the problem is that this happens during UI initialization.

But I looked at the stacktrace and stepped through the code, and it look like tools start out enabled, and they only have to be disabled by this code when (simplifying a lot, I haven't even tried to understand the exact conditions) they can't in fact be applied to the current selection (e.g., you can't bold an image). The selection starts out as a cursor at the beginning of the text, unless there's no sane text to put the cursor into; which means that most tools should stay enabled on most pages, and the problematic code never runs. What page were you testing on?

How do I get these pretty profiles? My dev tools profiler doesn't show "Recalculate Style" as a separate item anywhere.

Also, 20 milliseconds is not what I'd call massive…

I think 20ms is massive.

In T89423#1044418, @ori wrote:

In T89423#1037020, @matmarex wrote:

What page were you testing on?

http://osmium/wiki/Barack_Obama

To reach it, you'll have to set up a SOCKS proxy to the production cluster. Let me know if you need help with that.

Which presumably would require him to have some sort of existing access to the production cluster?

Indeed, I don't have access to production (and don't really want it). I'll assume that you basically tested on en.wp, dig in and see what happens, but this task doesn't really look actionable to me.

Good news! I have discovered that we accidentally disable and then re-enable pretty much the entire toolbar while the page loads. Getting rid of that should cut down on these pesky setDisabled() calls.

• ve.init.mw.Target.prototype.onReady
  • this.setupSurface( … ) → ve.init.mw.Target.prototype.setupSurface
    • target.setSurface( surface ) → ve.init.Target.prototype.setSurface
      • this.setupToolbar( surface ) → ve.init.mw.ViewPageTarget.prototype.setupToolbar
        • target.getToolbar().initialize() → ve.ui.Toolbar.prototype.initialize
          • this.updateToolState() – at this point, the surface has no selection (and isn't focused), so the tools can't be applied, so they get disabled
    • target.emit( 'surfaceReady' ) → ve.init.mw.ViewPageTarget.prototype.onSurfaceReady
      • this.getSurface().getView().focus() – since there is no selection yet, one will be created, which emits a contextChange event, which goes to ve.ui.Toolbar.prototype.onContextChange and again this.updateToolState() – there is a selection now, applicable tools get re-enabled

Possibly we could just remove the .updateToolState() call in ve.ui.Toolbar.prototype.initialize (added in 21371ce846f72e3d249557f4a2939b8b2fe2c273 / https://gerrit.wikimedia.org/r/#/c/129806/). But not sure how that would affect non-MW uses of VE, and @Esanders and @Catrope probably had a good reason to add it (it was even cherry-picked to prod back then, sadly no records exist about why that was done).

what is the order of things in core?

Change 191378 had a related patch set uploaded (by Bartosz Dziewoński):
ve.ui.Toolbar: Don't call #updateToolState from #initialize

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

Patch-For-Review

Standalone has the same order – first setSurface() is called, calling Toolbar#initialize, which disables everything because there's no selection (NullSelection); then the surface is focused, emits contextChange, which enables everything (other than tools which don't apply, so basically only table cell and header format tools in most cases).

Just be careful when writing a fix here because the correct initial select in the model is in fact NullSelection. We then attempt to find somewhere to place the cursor initially, but if your document only contains an image, then the selection will stay null and no contextChange will fire. If we just removed the updateToolState form initialize it wouldn't get called at all in this edge case (I think).

The worst case is that toolbar icons will not be disabled when they should, until the user clicks the surface themselves. But that's not going to happen anyway because, as far as I can tell, it's completely impossible to get a NullSelection after calling surface.focus() – there's always a slug or regular text node to put the cursor into.

Change 191378 merged by jenkins-bot:
ve.ui.Toolbar: Don't call #updateToolState from #initialize

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