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

Dirty dot no longer appears when JS or server file is edited #5317

Closed
njx opened this issue Sep 24, 2013 · 7 comments
Closed

Dirty dot no longer appears when JS or server file is edited #5317

njx opened this issue Sep 24, 2013 · 7 comments
Assignees
Milestone

Comments

@njx
Copy link
Contributor

njx commented Sep 24, 2013

  1. Create an HTML file that has a <script> tag referring to a JS file
  2. Open the HTML file in live preview
  3. Edit the JS file

Result: No dirty dot showing that the live preview is out of sync. This used to work. It's also broken for HTML files that are served from a user server (where live editing isn't enabled).

@ghost ghost assigned njx Sep 30, 2013
@njx
Copy link
Contributor Author

njx commented Sep 30, 2013

Reviewed. To me, medium priority

@njx
Copy link
Contributor Author

njx commented Nov 16, 2013

Nominating for sprint 35

@njx
Copy link
Contributor Author

njx commented Nov 27, 2013

It looks like the issue is that in LiveDevelopment.js, doc.url is no longer being set, so when _onDirtyFlagChange() tries to check if the current document has been requested by the browser, it passes undefined.

doc.url used to be set in setDocInfo(), but that function was ripped out in 1f6b5aa back when we added the ServerRequestManager stuff, and it looks like the code for setting doc.url was never migrated anywhere else. doc.url still gets used various places, so I'm surprised that other stuff isn't broken. Maybe this could be the cause of some of the other flakiness we see when switching documents, etc.

@jasonsanjose - it looks like the correct way to get the url for a document now is to call _server.pathToUrl() on the document's path - is that right? Could you take a look at the current uses of doc.url and see if that's appropriate for all of them, or if there's some other reason they didn't get migrated?

(As a side note, it looks like the comment for _getCurrentDocument() is incorrect now since it still claims that we're adding the extension/url/root to the document object.)

@njx
Copy link
Contributor Author

njx commented Nov 27, 2013

@jasonsanjose - I went ahead and tried replacing doc.url with calls to _server.pathToUrl() and it seems to fix this problem, but I'm unsure as to whether _server will always be available at the times the URL is needed. Could you look at the nj/issue-5317 branch and let me know what you think? https://github.com/adobe/brackets/compare/nj;issue-5317

@jasonsanjose
Copy link
Member

Those changes look good. The server is ready before the inspector is connected.

Thanks for finding that bug, sorry that I missed those doc.url refs. setDocInfo moved down into BaseServer.add() when we add live documents. We probably never treated .js files as live docs, but we gave them URLs...maybe unintentionally?

@njx
Copy link
Contributor Author

njx commented Nov 27, 2013

Yeah, not sure. It seems like we rely on URLs even for non-live docs, though, at least in this case.

@jasonsanjose
Copy link
Member

Confirmed fixed. Closing.

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

2 participants