Live Preview fixes #6889
Live Preview fixes #6889
Conversation
| @@ -45,6 +45,9 @@ define(function ConsoleAgent(require, exports, module) { | |||
| level = "warn"; | |||
| } | |||
| var text = "ConsoleAgent: " + message.text; | |||
| if (message.url) { | |||
| text += " (url: " + message.url + ")"; | |||
| } | |||
There was a problem hiding this comment.
The 404 error message provides a url that is not added to error message. This case I saw ended up not being related to this code, but this is good info to display.
|
@jasonsanjose Do you want to take a look? I think you were the last one to update this code. |
| if (_openDeferred.state() === "pending") { | ||
| _openDeferred.reject(); | ||
| } | ||
|
|
||
| _doInspectorDisconnect(doCloseWindow).done(cleanup); |
There was a problem hiding this comment.
Seems like a pending _openDeferred should be rejected before disconnecting. This makes the scenario I was using seem to be more reliable when switching files.
|
@jasonsanjose Re-assigned to @ingorichter |
|
@ingorichter @jasonsanjose Can someone make a first pass through this pull request? We should try to get this in for Release 37, and I have a few questions that still need to be resolved. Thanks. |
| @@ -589,14 +599,22 @@ define(function LiveDevelopment(require, exports, module) { | |||
| _setStatus(status); | |||
|
|
|||
| result.resolve(); | |||
| _loadAgentsPromise = null; | |||
There was a problem hiding this comment.
Remove each addition of _loadAgentsPromise = null; and instead setup the following:
result.always(function () {
_loadAgentsPromise = null;
}
There was a problem hiding this comment.
Much better. Thanks.
|
@redmunds initial review complete |
| reconnect().done(function () { | ||
| // Reload HTML page | ||
| Inspector.Page.reload(); | ||
| }); |
There was a problem hiding this comment.
In theory, the _onDocumentSaved() event handler could be called again before agents and page are done, but this is probably safe for now since I can't figure out how to hit this code. HTML adn CSS pages have liveEditingEnabled set to true, so the bail out. I tried attaching a JS file, but agents.network.wasURLRequested() returns falsey.
| // Agents are already loading, so don't unload | ||
| return _loadAgentsPromise; | ||
| } | ||
|
|
There was a problem hiding this comment.
@jasonsanjose I added better handling of unloadAgents().
| @@ -22,7 +22,7 @@ | |||
| */ | |||
|
|
|||
| /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, forin: true, maxerr: 50, regexp: true */ | |||
| /*global define, $, brackets, window */ | |||
| /*global define, $, brackets, window, open */ | |||
There was a problem hiding this comment.
New call to open() causes a circular dependency.
|
@jasonsanjose Ready for another review. |
|
Switching the documents will start a new session every time. This seems to be alright now. |
|
@ingorichter That last issue is worth filing - the fact that we get easily confused if you disconnect/reconnect rapidly. Some of the students were running into this at MissionBit. |
| @@ -1013,10 +1038,8 @@ define(function LiveDevelopment(require, exports, module) { | |||
| _close() | |||
There was a problem hiding this comment.
I'm questioning now whether or not this is useful. At this point, we've spun up the server and created the main live document, but we just haven't established a connection to Chrome. Either it's not running or we can't connect to the debug port. Does this call to close even make sense?
If not, then I think we can remove it and your change below goes back to _openInterstitialPage without the timeout as you said.
|
Unfortunately, I can still hit the "Unable to load Live Development page" error message following this comment #6889 (comment). Is it worth it to try changing |
…avigate() instead and update agent and server state as needed.
Do not close/reopen live preview on currentDocumentChange
|
Looks great. Merging. |
This is for #6122. Also is for #5687. And at least parts of #6830.
I also found a repeatable recipe that was causing connection errors when changing files, so I fixed the case of relaunching browser after "Relaunch Chrome" dialog is shown. See inline comments for more info.