search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

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

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by redmunds Wednesday Jan 14, 2015 at 19:24 GMT Originally opened as https://github.com/adobe/brackets/issues/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

core-ai-bot commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

@busykai Ah, yes. Nevermind :)