Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test "EditorView.movePos / can cross large line widgets during line motion" is failing #71

Closed
marijnh opened this Issue Jan 16, 2019 · 18 comments

Comments

Projects
None yet
2 participants
@marijnh
Copy link
Member

marijnh commented Jan 16, 2019

I noticed and will look into it tomorrow!

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Jan 16, 2019

Works for me in Firefox, but EditorView extension / calls update when the viewport changes fails for me :)

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Jan 16, 2019

That was failing earlier—are you up to date with master?

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Jan 16, 2019

master points to a commit from me, so yeah, I'm up to date.

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Jan 16, 2019

Weird. I'll see what I find.

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Jan 17, 2019

The problem was introduced when my changes made the tests actually use our CSS (they weren't importing editorview.css before) and actually exposed an existing problem that was randomly not triggered before, and only happened for low window heights (when I had my dev tools open). I've fixed that in 40a1150.

I can not get the test you mention to fail, so I can't investigate that.

@marijnh marijnh closed this Jan 17, 2019

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Feb 13, 2019

Hm, it still consistently fails for me in Firefox 65:

Error: 1 != 2 @view/test/test_built.js:7606:13ist_1</ist.Failure@view/test/test_built.js:6886:22
ist@view/test/test_built.js:6876:17
@view/test/test_built.js:7606:13
@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Feb 13, 2019

I'm going to need to rewrite some of the related code anyway in my (local, way too long-lived) branch around #76 . Reopening, let's take another look when that lands.

@marijnh marijnh reopened this Feb 13, 2019

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Feb 15, 2019

This is now passing for me again. How about you?

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Feb 15, 2019

No change; I suppose I have to investigate that myself :)

@marijnh marijnh closed this in d1fb878 Mar 13, 2019

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Mar 13, 2019

I got it to reproduce in Chrome when the devtools were closed, and attached patch should solve this. See also https://discuss.codemirror.net/t/cm6-failing-unit-test-cross-large-line-widgets-on-macos-chrome/2020

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Mar 14, 2019

No change for me. posAtCoords is not even executed during that test.

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Mar 14, 2019

Well this keeps getting more interesting

@marijnh marijnh reopened this Mar 14, 2019

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Mar 14, 2019

Yeah :) Can you give me a stack trace for posAtCoords in that test? I'd like to see where things go wrong for me.

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Mar 14, 2019

movePos, in the if (granularity == "line") case, will call posAtCoords in the loop. You are using Firefox, right? Could it be that browser.gecko is false because you're using an oddball build with an unexpected user agent string?

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Mar 14, 2019

movePos is not executed either, and browser.gecko is true.

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Mar 14, 2019

Uhm, the test case failing for me is EditorView extension / calls update when the viewport changes, not the one in the issue title, right?

@marijnh

This comment has been minimized.

Copy link
Member Author

marijnh commented Mar 14, 2019

Oh, I assumed we were talking about the one in the title. No, there I don't think posAtCoords is relevant.

@adrianheine

This comment has been minimized.

Copy link
Collaborator

adrianheine commented Mar 14, 2019

Yeah, no, sorry for the confusion. The one in the title always passed for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.