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

Fix for calling Native.closeLiveBrowser() when turning off Live Development. #5392

Closed
wants to merge 6 commits into from
Closed

Fix for calling Native.closeLiveBrowser() when turning off Live Development. #5392

wants to merge 6 commits into from

Conversation

fungl164
Copy link
Contributor

@fungl164 fungl164 commented Oct 1, 2013

This requires brackets-shell pull request: adobe/brackets-shell#339

});
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have this code fixed so unit tests pass for me. Here's what I did:

  1. This function is using wrong promise, so I updated it to take a param:
    function _closeLiveBrowser(deferred) {
        NativeApp.closeLiveBrowser()
            .done(function () {
                _setStatus(STATUS_INACTIVE, reason || "explicit_close");
                deferred.resolve();
            })
            .fail(function (err) {
                _setStatus(STATUS_ERROR);
                deferred.reject("CLOSE_LIVE_BROWSER");
            });
    }

function _closeLiveBrowser(deferred) {
NativeApp.closeLiveBrowser()
.done(function () {
_setStatus(STATUS_INACTIVE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be: _setStatus(STATUS_INACTIVE, reason || "explicit_close");

This function needs to be moved inside of the _close() function closure so it gets the reason variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

@redmunds
Copy link
Contributor

redmunds commented Oct 3, 2013

Everything is working great! Just some code cleanup to do in this branch.

@fungl164
Copy link
Contributor Author

fungl164 commented Oct 3, 2013

@redmunds FYI, made changes to have Live Dev connection launch only when the LiveBrowser instance is up and running instead of polling for connection failures.

@redmunds
Copy link
Contributor

@fungl164 OK, I have time to look at this again. I mentioned skinning the live-dev-profile just as a way to remind user to open new tabs/windows in a different profile -- I don't think it's the final solution. Once again, here's the scenario:

  1. Close all Chrome windows
  2. Start Live Dev
  3. At this point, there is only a single Chrome window open

On Windows, if I use New > Window in Chrome menu, I can see that the new (second) Chrome window does not open in my default profile (which has a custom skin). I asusme it's live-dev-profile. When I close Live Dev, only the window opened by Brackets is closed -- not both windows. I'd like to get this same behavior on Mac.

Question: Following Step 3 above, how do you open a new window in the default profile? I can't figure out how to do it using Chrome menus. If there is a reasonable way that we can tell Brackets users (or even better, they can figure out on their own), then this might be acceptable. I know how to do it using the command line, but I don't think this is acceptable.

@redmunds
Copy link
Contributor

@fungl164 I demo'ed this functionality to the core team today and they were very excited -- as you know, this is a real sore point with Mac users.

We decided that we don't want to take this code until we can fix the issue with closing windows that user has manually opened. We're grateful for the contribution you have made so far. Let me know if you want to continue working on this, but if not that's ok and I can merge this into a branch on the brackets so we can continue to work on it. Thanks.

@fungl164
Copy link
Contributor Author

I totally understand. It was a sore point for me. But no more. :)

I'm glad to hear there is excitement around this. If I can get the info I need (see https://github.com/adobe/brackets-shell/pull/339/files#r6919793), I may be able to get it working as you envision it.

@redmunds
Copy link
Contributor

I remembered that there's an extension called Theseus on the Brackets Registry meant to help with debugging async bugs like this. I haven't figured anything out yet, but thought that it may help you.

@fungl164
Copy link
Contributor Author

New pull request created (see #5569)

@redmunds
Copy link
Contributor

This has been superseded by #5569. Closing.

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.

2 participants