[CLOSED] Live Preview MultiBrowser fails to connect in this case

[core-ai-bot]

Issue by redmunds
Wednesday Jan 14, 2015 at 19:24 GMT
Originally opened as adobe/brackets#10374

---

Live Preview MultiBrowser fails to connect in this case. More details in these other comments in PR #10285

1. Start Bracket s on a system with a local server setup and running
2. Switch to a site with files on the local server
3. I originally hit this by setting base url to be http:/localhost/, but this is not required
4. Switch to LiveDev MultiBrowser
5. Start Live Dev

Results
Live Dev never completely connects. It stays in Connecting state.

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 04, 2015 at 21:49 GMT

---

@redmunds@sebaslv Isn't the root problem here that Multi-Browser doesn't support custom servers? (also alluded to in #10217)

Is the idea that Multi-Browser should just ignore the base URL and load the page statically as if no URL was set? At a glance that seems like the sort of behavior PR #10453 is shooting for.

Getting it to actually respect a custom server is a much more complex undertaking that I don't think we have a good answer for yet, right? For the same reason that we can't do HTML live updating on user servers with the 'classic' Live Preview impl (see Trello card): we can't inject content into the HTML page without having some of our code in the server side.

[core-ai-bot]

Comment by sebaslv
Thursday Mar 05, 2015 at 17:35 GMT

---

Yes, the root cause is that MB doesn't support custom servers but, what makes this issue being manifested is that servers registered by LiveDevelopment are not being cleaned-up when switching to MB which makes LiveDevMultiBrowser not able to get StaticServer from LiveDevServerManager. It returns UserServerbecause of priority if URL was set before. StaticManager is needed by MB to make instrumentation possible.

This PR proposes to solve this issue by extending LiveDevServerManager to let LiveDevelopment register/remove the servers when switching btw implementations so only StaticServer keeps registered when running MB.

It follows@redmunds's recommendation from PR #10285.

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 11, 2015 at 23:54 GMT

---

I wonder what the "correct" behavior is when the user has a custom server URL set, though. Silently serving up the page statically instead seems confusing -- maybe we should show a warning dialog the first time (per project) that Multi-Browser Live Preview is launched while a server URL (that it will ignore) is in effect.

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 11, 2015 at 23:55 GMT

---

Also -- I take back one part of what I said in my first comment. Solving this Multi-Browser limitation could be a good deal simpler than getting Live HTML to work with a custom server. Since all we need to do for Multi-Browser is inject a single <script> tag, at least two simpler options would work:

• Interject our own proxy server in front of the user's custom server, and have the browser go to our URL. Our proxy injects the tag on the fly. (Thanks to@njx for pointing this out). This could still break some users' projects where the domain name the browser sees is important (e.g. pages making CORS requests), but it would work for most people.
• Provide users a bookmarklet that they can click on in their browser to add the instrumentation (Weinre does this). We miss the initial page-load related events, but I don't think we currently need them, do we? (The bigger downside is it's just more of a pain for users, and more of a pain for us to build a UI & docs that explain how to do it).

[core-ai-bot]

Comment by busykai
Monday Apr 13, 2015 at 19:56 GMT

---

@peterflynn,@sebaslv I think we're dealing with two issues here:

1. Confusion and actual errors (as the one described in this issue) due to the fact that multi-browser does not support UserServer.
2. The fact that multi-browser implementation does not support UserServer configuration.@peterflynn's later comments outline some strategies on how it can be fixed with lesser effort.

I think PR #10453 is an adequate fix for 1). Item 2) is a separate issue and needs to be addressed as such.

So I'm going to review and merge #10453 and start the work on #10217 (which is medium priority, but I haven't got a chance to advance on it at all)...

[core-ai-bot]

Comment by redmunds
Thursday May 14, 2015 at 17:59 GMT

---

@busykai Looks like this was auto-closed. Should it be re-opened for the other half?

[core-ai-bot]

Comment by busykai
Thursday May 14, 2015 at 18:08 GMT

---

@redmunds, we have #10217 to track the user server case (this is the part that is not fixed, right?).

[core-ai-bot]

Comment by redmunds
Thursday May 14, 2015 at 18:10 GMT

---

@busykai Ah, yes. Nevermind :)