Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Switching to JS file while Live Development is connecting causes it to fail #5110

Open
peterflynn opened this issue Sep 6, 2013 · 18 comments

Comments

@peterflynn
Copy link
Member

I could swear we fixed a bug just like this recently, but this occurs for me on master now...

  1. Open an HTML file and a JS file in the working set
  2. Go to the HTML file and click the Live Preview lightning bolt
  3. While Chrome is launching, switch to the JS file

Result:
Chrome stays on the gray "interstitial" page forever, and Brackets eventually shows a connection error dialog.

Expected:
Should still connect, since I was on a valid HTML file when I originally clicked it.
And if I wait on the HTML file until the connection succeeds, switching to the JS file at that point doesn't cause it to disconnect... so we're putting the user in a sort of race condition.

This is easy to hit if you're using Live Preview but what you actually want to work on is a CSS or JS file. You can't launch it from the file you're in (because of the "you must select an HTML file first" issue), so you quickly switch to the HTML file and then back to the file you actually care about.

Also, it's easier to hit on Win than Mac, because every Live Preview invocation means waiting for an instance of Chrome to launch from scratch -- much longer time period in which to hit the bug.

@peterflynn
Copy link
Member Author

This didn't really work in earlier sprints either -- in Sprint 29, it causes Chrome to show the JS file's source in the browser tab, and in Sprint 30 the current bug (above) repros. So maybe not a high priority...

@lkcampbell
Copy link
Contributor

@peterflynn, I've actually run into this issue a few times myself, performing the exact scenario you describe above. When I did it last, I was using Sprint 29, and I thought the display of the source code was an odd, but intentional, way to address the problem.

Your Expected result make a lot more sense though. Especially since this behavior is now causing an error in the connection.

@ghost ghost assigned couzteau Sep 16, 2013
@njx
Copy link
Contributor

njx commented Sep 16, 2013

Medium priority to @couzteau

@couzteau
Copy link
Member

I observed reviewing the launch code that the helper function _doLaunchAfterServerReady is calling _getCurrentDocument again - after the open function has already retrieved a ref top the current document.

I have set up a branch with a fix: jhagenst/fix-5110

I think the fix is good, but I have only seen a small fraction of the LiveDebugger code, hence I have the feeling I don't have sufficient insight. Thanks for a comment on wether the fix seems to do the right thing.

@couzteau
Copy link
Member

posted PR #5268

@njx
Copy link
Contributor

njx commented Sep 24, 2013

FBNC to @peterflynn

@ghost ghost assigned peterflynn Sep 24, 2013
@peterflynn
Copy link
Member Author

Waiting until the "index.html-finding" code lands before regressing this

@pthiess
Copy link
Contributor

pthiess commented Oct 7, 2013

@peterflynn Please regress.

@peterflynn
Copy link
Member Author

@couzteau When I follow the repro steps now, Chrome closes when I do step 3. I would expect it to continue connecting to the HTML file to that the outcome is the same as if I waited a few seconds more before clicking the JS file.

This is less nasty than getting stuck on the loading screen forever, so changing priority to Low... but reopening. Leaving Sprint unassigned for now.

@ghost ghost assigned couzteau Oct 9, 2013
@peterflynn
Copy link
Member Author

Btw, it's possible this was working better when @couzteau's PR #5268 landed, but was broken subsequently when #5414 landed.

@couzteau
Copy link
Member

couzteau commented Oct 9, 2013

@peterflynn I cannot repro what you describe. Can you show me. I do agree that it is not unlikely that there is relationship to #5414

@redmunds
Copy link
Contributor

Added Brackets 1.0 milestone. This is similar to #5756 -- difference is that JS file is selected instead of HTML file.

@couzteau
Copy link
Member

I can repro now. In order to repro It's required that the HTML file launched in LivePreview is not named index.html

@peterflynn
Copy link
Member Author

Marking needs review to reconsider 1.0 in/out question later, once we know more about our 1.0 plans for the Live Preview architecture.

Also note: #4615 might be a dupe of this now that its description has been updated.

@redmunds
Copy link
Contributor

FBNC to @peterflynn . Switched Milestone from Brackets 1.0 to Release 40.

@redmunds redmunds modified the milestones: Release #40, Brackets 1.0 May 29, 2014
@redmunds
Copy link
Contributor

redmunds commented Jun 3, 2014

Forgot to re-assign to @peterflynn

@redmunds
Copy link
Contributor

@peterflynn ping

@redmunds
Copy link
Contributor

@peterflynn Can you verify this old FBNC issue?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants