Package tabs: disable view if clientX or clientY is negative

[asiloisad]

I'm the author of the pdf-viewer package. After switching to PulsarNext a strange exception appears.

Steps to reproduce:

1. open .pdf file in pdf-viewer
2. drag tab over right edge of Pulsar
3. exception occurs because "closest is not a function of undefined".

I have been investigating this a bit and have discovered something strange. Sometimes clientX and clientY of the drag event are negative, so document.elementFromPoint cannot find an element and returns undefined. In Pulsar (old) I never noticed this and the above statement does not throw an exception. Suggested solution has solved the problem, but I'm not sure if it's optimal. Perhaps a better solution would be to diagnose and solve the problem of why clientX and clientY take negative values.

[savetheclocktower]

Can you consistently reproduce the scenario where clientX and/or clientY is negative? I'm curious about what triggers it.

[asiloisad]

I had modify drag(e) function of layout.js file of tabs package to log e.clientX, e.clientY to dev console. The negative values are reported only if pdf-viewer pane is last right one and it tabs is dragged over right edge.

The video of my test is uploaded to yt: https://youtu.be/SP7BTbnyr98

I want to notice the pdf-viewer pacakge works fine on Pulsar, because it never reported negative values of clientX or clientY.

[asiloisad]

I have done more tests. In Pulsar 1.124.0 bugs appear too 🙁, but after window:reload stops appear. In PulsarNext 1.124 it look like editor is more responsible, so window:reload do not stop it. It's quite hard for me to debug pdf.js interaction with pulsar. I will be glad if this fix can go to master branch. It doesn't affect the program in normal circumanties after all.

[savetheclocktower]

@asiloisad Thanks for the video! That's helpful.

The initial issue is clear: clientX seems to go negative when the tab is dragged over the docked developer tools. Simple enough to fix.

Late in the video, it seems like the error is triggered when you drop the tab back in its existing pane container. Do I have that right? The first time it happens is around the 2:00 mark in the video. It's weird because it seems like clientX and clientY are valid values until the very last drag event and dragend event, hence the two exceptions that suddenly appear in the dev tools.

Anyway, I'm just curious. I might apply this fix in a slightly different way, but I'll make sure it lands. Thanks!

[savetheclocktower]

@asiloisad, I think 78be47c might fix this. My thinking is that we just have to prevent the exception here — that way getElement will return undefined (as it should) and the right code path will be hit.

I pushed that commit to updated-latest-electron; let me know if it works for you. (Sorry, should've pushed this and the other commit at the same time. :))

[asiloisad]

You are right. At 1:59 there is second case which lead to negative clientX. I have hit Esc at the end of drag, which raise an exception.

There is one more pdf package based on pdf.js in pulsar repository. It has the same problem as my package... :(

Your fix works fine as well. Thank you! I will work more with pdf.js in future to improve my package.

[savetheclocktower]

I'll reopen this because we ought to backport the fix to the stable release. I'll close it again once that lands.