-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Fix live dev issues after ResearchLiveHTML merge #4975
Conversation
…cument before starting a new live dev session. fix server cleanup remove extraneous runs() block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
off("activeEdtiorChange"...) was missing. We didn't hit this prior to #4801 since we were always changing the live document when currentDocumentChanged fired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here--should remove the .CSSDocument namespace from EditorManager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for there to be more than one CSSDocument open at a time? If so, then this would remove all handlers for all CSSDocuments when one of them closes (either what's currently in the code or if you change it to just removing the namespace).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(That issue doesn't apply to the $(this.doc) handlers since presumably there's only one CSSDocument per Document.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSSDocument is 1:1 with Document. @njx is right that this will break when there are more than once CSSDocuments open. I'll fix.
|
@jasonsanjose This branch fixes all of the problems I was seeing on win7. |
|
@redmunds Can you review the base URL prompt code in LiveDevelopment.js, lines 909-940? I took a look, but it would be good to get another set of eyes on it. |
|
@gruehle I will review URL prompt code. |
|
@gruehle pushed fixes |
|
Review complete. @redmunds feel free to merge once you've reviewed the URL prompt code. |
|
Looks good. Merging. |
Fix live dev issues after ResearchLiveHTML merge
Replaces pull #4969
Fixes #4966 and #4972.
Fixes failing unit "in-memory" live dev test where event handlers were not removed properly and caused an error in
RemoteAgentwhen trying to communicate with a closed Inspector connection.