Fix #3372 check for valid server #3392

Merged
merged 5 commits into from Apr 10, 2013

Projects

None yet

2 participants

@jasonsanjose
Member

Fix #3372

Also, made a small refactoring to live dev startup to create live document before agents load.

@jasonsanjose jasonsanjose commented on the diff Apr 9, 2013
src/LiveDevelopment/LiveDevelopment.js
@@ -572,23 +603,7 @@ define(function LiveDevelopment(require, exports, module) {
function doLaunchAfterServerReady() {
_setStatus(STATUS_CONNECTING);
- if (_serverProvider) {
- // Install a request filter for the current document. In the future,
- // we need to install filters for *all* files that need to be instrumented.
- HTMLInstrumentation.scanDocument(doc);
- _serverProvider.setRequestFilterPaths(
- ["/" + ProjectManager.makeProjectRelativeIfPossible(doc.file.fullPath)]
- );
-
- // Remove any "request" listeners that were added previously
- $(_serverProvider).off(".livedev");
-
- $(_serverProvider).on("request.livedev", function (event, request) {
- var html = HTMLInstrumentation.generateInstrumentedHTML(doc);
-
- request.send({ body: html });
@jasonsanjose
jasonsanjose Apr 9, 2013 Adobe Systems Incorporated member

Created a new API getResponseData() for live documents instead of hard coding HTMLInstrumentation here.

@jasonsanjose jasonsanjose commented on the diff Apr 9, 2013
src/LiveDevelopment/LiveDevelopment.js
+ // Send custom HTTP response for the current live document
+ $(_serverProvider).on("request.livedev", function (event, request) {
+ var response = _liveDocument.getResponseData ? _liveDocument.getResponseData() : null;
+ request.send(response);
+ });
+ }
+
+ _liveDocument.setInstrumentationEnabled(enableInstrumentation);
+ }
+ }
+
+ /**
+ * @private
+ * Populate array of related documents reported by the browser agent(s)
+ */
+ function _getRelatedDocuments() {
@jasonsanjose
jasonsanjose Apr 9, 2013 Adobe Systems Incorporated member

Separate related documents from the current document (_liveDocument). This allows us to create the HTMLDocument earlier, before the agents load. Computing the related documents requires the root HTML file and it's agents to load.

@jasonsanjose jasonsanjose commented on the diff Apr 9, 2013
src/LiveDevelopment/LiveDevelopment.js
+ // Enable instrumentation
+ if (_liveDocument.setInstrumentationEnabled) {
+ var enableInstrumentation = false;
+
+ if (_serverProvider && _serverProvider.setRequestFilterPaths) {
@jasonsanjose
jasonsanjose Apr 9, 2013 Adobe Systems Incorporated member

Primary fix for #3372 to check for setRequestFilterPaths.

@jasonsanjose jasonsanjose and 1 other commented on an outdated diff Apr 9, 2013
src/LiveDevelopment/LiveDevelopment.js
+ // Enable instrumentation
+ if (_liveDocument.setInstrumentationEnabled) {
+ var enableInstrumentation = false;
+
+ if (_serverProvider && _serverProvider.setRequestFilterPaths) {
+ enableInstrumentation = true;
+
+ _serverProvider.setRequestFilterPaths(
+ ["/" + encodeURI(ProjectManager.makeProjectRelativeIfPossible(doc.file.fullPath))]
+ );
+
+ // Send custom HTTP response for the current live document
+ $(_serverProvider).on("request.livedev", function (event, request) {
+ var response = _liveDocument.getResponseData ? _liveDocument.getResponseData() : null;
+ request.send(response);
@jasonsanjose
jasonsanjose Apr 9, 2013 Adobe Systems Incorporated member

response can be null in which case the StaticServerDomain reverts to simple file serving.

@redmunds
redmunds Apr 10, 2013 collaborator

This would be a good comment to put in the code.

@jasonsanjose
jasonsanjose Apr 10, 2013 Adobe Systems Incorporated member

fixed

@jasonsanjose jasonsanjose commented on the diff Apr 9, 2013
src/LiveDevelopment/LiveDevelopment.js
+ // Enable instrumentation
+ if (_liveDocument.setInstrumentationEnabled) {
+ var enableInstrumentation = false;
+
+ if (_serverProvider && _serverProvider.setRequestFilterPaths) {
+ enableInstrumentation = true;
+
+ _serverProvider.setRequestFilterPaths(
+ ["/" + encodeURI(ProjectManager.makeProjectRelativeIfPossible(doc.file.fullPath))]
@jasonsanjose
jasonsanjose Apr 9, 2013 Adobe Systems Incorporated member

Bug fix for paths like /samples/root/Getting Started/index.html. Without encoding, the location.pathname does not match.

@redmunds redmunds and 1 other commented on an outdated diff Apr 10, 2013
src/LiveDevelopment/Documents/HTMLDocument.js
@@ -80,6 +78,34 @@ define(function HTMLDocumentModule(require, exports, module) {
$(this.editor).on("change", this.onChange);
}
};
+
+ /**
+ * Enable instrumented HTML
+ * @param enabled {boolean}
+ */
+ HTMLDocument.prototype.setInstrumentationEnabled = function setInstrumentationEnabled(enabled) {
+ if (enabled && !this._instrumentationEnabled) {
+ HTMLInstrumentation.scanDocument(this.doc);
+ HTMLInstrumentation._markText(this.editor);
+ }
+
+ this._instrumentationEnabled = enabled;
+ };
+
+ HTMLDocument.prototype._instrumentationEnabled = false;
@redmunds
redmunds Apr 10, 2013 collaborator

HTMLDocument.prototype._instrumentationEnabled is not used. This works because this._instrumentationEnabled is initially undefined, which is falsy, but you should instead initialize this._instrumentationEnabled in the constructor.

@jasonsanjose
jasonsanjose Apr 10, 2013 Adobe Systems Incorporated member

fixed

@redmunds
Collaborator

Done with initial review.

@jasonsanjose
Member

Fixes pushed

@redmunds redmunds was assigned Apr 10, 2013
@jasonsanjose
Member

Added a fix for #3394 to clear out old filter requests.

@redmunds
Collaborator

Verified #3394 is fixed.

@redmunds
Collaborator

Looks good. Merging.

@redmunds redmunds merged commit 6f2dabd into master Apr 10, 2013

1 check passed

Details default The Travis build passed
@redmunds redmunds deleted the jasonsanjose/issue-3372 branch Apr 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment