[Regression wmf.16] Toolbar is not floating as I scroll down in VE, Error in the console "Uncaught TypeError: Cannot read property 'center' of undefined"
Closed, ResolvedPublic1 Story Points

Description

Toolbar is not floating as I scroll down in VE, Error in the console "Uncaught TypeError: Cannot read property 'center' of undefined"

Ryasmeen created this task.Mar 10 2017, 7:19 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 10 2017, 7:19 PM

What browser is this in? I can't replicate on Chrome.

Ryasmeen added a comment.EditedMar 11 2017, 1:07 AM

It is in chrome, however you need to select a focusable node first and then scroll to get the error first time. After that, it will keep showing that error even if its not selected.

Jdforrester-WMF renamed this task from [Regression pre-wmf.16] Toolbar is not floating as I scroll down in VE, Error in the console "Uncaught TypeError: Cannot read property 'center' of undefined" to [Regression wmf.16] Toolbar is not floating as I scroll down in VE, Error in the console "Uncaught TypeError: Cannot read property 'center' of undefined" .Mar 16 2017, 2:41 AM
Jdforrester-WMF triaged this task as Unbreak Now! priority.
Restricted Application added subscribers: Jay8g, TerraCodes. · View Herald TranscriptMar 16 2017, 2:41 AM

I also can't get the JS error after I select a focussable node, even on https://en.wikipedia.beta.wmflabs.org/wiki/User:RYasmeen_(WMF)/sandbox?veaction=edit :-(

DLynch added a subscriber: DLynch.Mar 16 2017, 4:56 AM

I cannot persuade this to happen, either. Reading computerPosition, I don't see an obvious way for it to happen without really weird stuff occurring (i.e. the direction CSS property being something other than ltr or rtl on the popup $container).

@Ryasmeen: Could you say whether it's still happening for you, and provide a more explicit test-case for it if so? (Which element you're clicking on, etc.)

@DLynch: Sure, I was able to replicate it in both test2 and Beta on following pages:
https://test2.wikipedia.org/wiki/Universe
https://test2.wikipedia.org/wiki/User:RYasmeen_(WMF)
https://en.wikipedia.beta.wmflabs.org/wiki/User:RYasmeen_(WMF)/sandbox

So the steps are:

  1. Open VE for any of these pages. For example: https://en.wikipedia.beta.wmflabs.org/wiki/User:RYasmeen_(WMF)/sandbox
  2. After opening the page select the graph/gallery/map node.

3.Click on "Read" mode to close VE
4.Open VE again
5.Scroll

At this point, the error should appear in the console, the toolbar won't float.

The issue seems to be that when you leave-and-reenter VE, the popup instance persists, along with its copy of $container. But the $container it's using no longer exists, because it's the destroyed-surface from the previous VE session. As such, with a removed-from-DOM-and-thrown-away element, getting its CSS direction gives us an empty-string, which causes the error we see here.

Change 343217 had a related patch set uploaded (by DLynch):
[VisualEditor/VisualEditor] DesktopContext: on destroy, tell our popup to stop positioning

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

I was, in fact, slightly misleading in my previous statement. The popup instance is indeed thrown away, it's just that it sets up a scroll handler, and so exists in that context. The patch has it tear that down.

Change 343217 merged by jenkins-bot:
[VisualEditor/VisualEditor] DesktopContext: on destroy, tell our popup to stop positioning

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

Catrope added a subscriber: Catrope.EditedMar 17 2017, 12:31 AM

The issue seems to be that when you leave-and-reenter VE, the popup instance persists, along with its copy of $container. But the $container it's using no longer exists, because it's the destroyed-surface from the previous VE session. As such, with a removed-from-DOM-and-thrown-away element, getting its CSS direction gives us an empty-string, which causes the error we see here.

So maybe FloatableElement should check if $floatableContainer is still attached, and short-circuit if not? PopupWidget already does that (for the attachedness of the popup itself).

Change 343224 had a related patch set uploaded (by Jforrester):
[mediawiki/extensions/VisualEditor] Update VE core submodule to master (ac6d7031c)

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

Change 343224 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor] Update VE core submodule to master (ac6d7031c)

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

Change 343228 had a related patch set uploaded (by Jforrester; owner: DLynch):
[VisualEditor/VisualEditor] DesktopContext: on destroy, tell our popup to stop positioning

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

Change 343228 merged by jenkins-bot:
[VisualEditor/VisualEditor] DesktopContext: on destroy, tell our popup to stop positioning

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

Change 343248 had a related patch set uploaded (by DLynch):
[oojs/ui] FloatableElement: Abort positioning if no longer attached

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

Change 343248 merged by jenkins-bot:
[oojs/ui] FloatableElement: Abort positioning if no longer attached

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

Change 343411 had a related patch set uploaded (by Jforrester):
[mediawiki/extensions/VisualEditor] Update VE core submodule to wmf/1.29.0-wmf.16 HEAD (rGVEDf6e399eed0a2)

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

Change 343411 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor] Update VE core submodule to wmf/1.29.0-wmf.16 HEAD (rGVEDf6e399eed0a2)

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

Jdforrester-WMF closed this task as Resolved.Mar 17 2017, 11:37 PM
Jdforrester-WMF claimed this task.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMar 17 2017, 11:37 PM
Jdforrester-WMF set the point value for this task to 1.Mar 20 2017, 3:19 PM