getBoundingClientRect called on undefined in TextEditorComponent sometimes

[steelbrain]

Continuing from steelbrain/linter-ui-default#355

What we know so far

• The error comes from pixelPositionForMouseEvent
• The error is often from commands like core:confirm, core:copy etc
• Either scrollContainer or lineTiles is undefined in Text Editor when those are invoked and it's not handled in Atom internally

First seen on 1.19.0-beta0, still hitting on 1.19.0

My rough guess about what it could be (not dug deep into sources so might be very wrong):

• A race condition after switching pane, those refs aren't yet resolved when the user moves mouse
• Either of these is two occurrences has the object undefined when the text editor is out of focus before moving mouse because in reports, one user had just closed dev tools, other kept switching, closing/reopening pane items
• Maybe the pane component doesn't check if it's been disposed and the items no longer exist and the bug is actually in linter-ui-default requesting mouse positions for closed tabs? (Don't request mouse position if editor is disposed steelbrain/linter-ui-default#420 fixes linter-ui-default side)

[robjtede]

Definitely reproducible by myself and others on steelbrain/linter-ui-default#355

Sometimes, not always, when closing tabs, that error is raised through the linter-ui-default package even after it's update (steelbrain/linter-ui-default#420).

[nathansobo]

This is a private API but I'd still like to help out. I'm pretty confused because both of those refs should be defined after the editor is initially constructed and should never be nulled out.

[nathansobo]

@as-cii Do you have any ideas on this one. As far as I can tell both the scrollContainer and lineTiles refs are created on construction of the component and never go away.

[as-cii]

Prior to Atom 1.19.2 (specifically, before #15317), the lineTiles ref may have not been there if we hadn't performed the initial measurements, so I wonder if this problem got fixed as part of that pull-request. @robjtede @steelbrain: can you still reproduce this on the latest version?

[robjtede]

I haven't been able to reproduce the error since updating linter-ui-default to v1.6.7 (which was before updating Atom to 1.19.2).

[steelbrain]

@as-cii My best guess at this point is that linter-ui-default was requesting measurements when the text editor had been unmounted (we have some internal debounce logic) so in the latest update I've made sure not to ask for measurements if the editor is unmounted

That seems to have fixed it for people so far, as for the Atom side, I think the text editor refs are removed when the editor is closed? A safeguard that throws an error with a proper message would be nice and helpful in debugging further cases like this

[nathansobo]

From what I know about the TextEditorComponent and @as-cii's reminder about the change that landed in 1.19.2, I think what was happening is that the mousemove handler was calling that method on the editor before it had detected that it had become visible. Previously, we waited to render the lineTiles div until after performing an initial measurement of line height etc that requires the editor to be visible. Now we render the lineTiles div on construction, so the exception shouldn't occur. Thanks for the report.

[lock]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!