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

Conversation

@jasonsanjose
Copy link
Member

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 RemoteAgent when trying to communicate with a closed Inspector connection.

@ghost ghost assigned redmunds and gruehle Aug 28, 2013
@jasonsanjose
Copy link
Member Author

@gruehle: @redmunds found a good number of live dev connection bugs during scenario and smoke testing today. Most of them stem from #4801. I ran smoke, unit, integration and extension tests on mac and everything passes.

Copy link
Member Author

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.

Copy link
Member

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

Copy link

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).

Copy link

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.)

Copy link
Member Author

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.

@redmunds
Copy link
Contributor

@jasonsanjose This branch fixes all of the problems I was seeing on win7.

@gruehle
Copy link
Member

gruehle commented Aug 28, 2013

@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.

@redmunds
Copy link
Contributor

@gruehle I will review URL prompt code.

@jasonsanjose
Copy link
Member Author

@gruehle pushed fixes

@gruehle
Copy link
Member

gruehle commented Aug 28, 2013

Review complete. @redmunds feel free to merge once you've reviewed the URL prompt code.

@redmunds
Copy link
Contributor

Looks good. Merging.

redmunds added a commit that referenced this pull request Aug 28, 2013
Fix live dev issues after ResearchLiveHTML merge
@redmunds redmunds merged commit 60dc6b7 into master Aug 28, 2013
@redmunds redmunds deleted the jasonsanjose/issue-4966-rebase branch August 28, 2013 18:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Live Development does not update for currentDocumentChangeEvent

5 participants