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

Editor unresponsive when clicked on commented CSS while live preview is active #9002

Closed
liaujianjie opened this issue Sep 7, 2014 · 11 comments
Closed

Editor unresponsive when clicked on commented CSS while live preview is active #9002

liaujianjie opened this issue Sep 7, 2014 · 11 comments
Assignees
@peterflynn
Labels
fixed but not closed high priority needs unit test
Milestone
Release 0.44

Comments

@liaujianjie
Copy link

In release 43 (OSX Mavericks), the editor becomes unresponsive when you click on a multi-line comment (that comments out a valid class/id) in CSS while having the live preview running. I tried replicating the bug without plugins and it still occurs.
@RaymondLim RaymondLim added this to the Brackets 1.0 (#0.44) milestone Sep 7, 2014
@RaymondLim RaymondLim self-assigned this Sep 7, 2014
@Mark-Simulacrum
Copy link
Contributor

@RaymondLim Cannot reproduce unresponsiveness on 64bit Ubuntu 14.04. (on master ece9bbf).

I do see a 'Unable to load live preview page' dialog; but this happens after the index.html of the Getting Started project has rendered, although the Brackets' console says this (full stack trace below), but I'm not sure what this means - other than that would probably be the file that Brackets' errors on.

GET http://127.0.0.1:9222/json  Inspector.js:266
    getDebuggableWindows Inspector.js:266
    connectToURL Inspector.js:348
    _openInterstitialPage LiveDevelopment.js:1125
    _doLaunchAfterServerReady LiveDevelopment.js:1242
    (anonymous function) LiveDevelopment.js:1343
    j jquery-2.1.0.min.js:2
    k.fireWith jquery-2.1.0.min.js:2
    e.(anonymous function) jquery-2.1.0.min.js:2
    (anonymous function) jquery-2.1.0.min.js:2
    j jquery-2.1.0.min.js:2
    k.fireWith jquery-2.1.0.min.js:2
    e.(anonymous function) jquery-2.1.0.min.js:2
    onSuccess StaticServer.js:124
    (anonymous function) StaticServer.js:149
    j jquery-2.1.0.min.js:2
    k.fireWith jquery-2.1.0.min.js:2
    NodeConnection._receive

@marcelgerber
Copy link
Contributor

@Mark-Simulacrum That's a rather common error message, which is probably not part of the issue.
Apparently nobody knows what it means :D See #8611.

@marcelgerber
Copy link
Contributor

@Mark-Simulacrum I found a more reproducible recipe that works for me on Win. Could you try it?

  1. Create the two files i.html and i.css, with the contents:
<html>
    <head>
        <link rel="stylesheet" href="i.css" type="text/css">
    </head>
    <body>
        Some text.
    </body>
</html>
/* body {
    background-color: black;
    color: white;
} */
  1. Start live preview on i.html and wait
  2. Switch to i.css
  3. Remove */ on line 4, then press Backspace again

Result: The editor becomes unresponsive

@marcelgerber
Copy link
Contributor

git bisect points to @RaymondLim's c954fb4.

@RaymondLim RaymondLim mentioned this issue Sep 7, 2014
Merged
@Mark-Simulacrum
Copy link
Contributor

@marcelgerber Yes, your test case does allow me to reproduce the hang. Will test the PR branch soon.

@RaymondLim RaymondLim mentioned this issue Sep 14, 2014
Closed
@peterflynn peterflynn self-assigned this Sep 17, 2014
@peterflynn
Copy link
Member

@Mark-Simulacrum @marcelgerber Since you both repro'ed this earlier, would one of you mind confirming the fix before we close it? (I tested it a bit during PR review, but this was a nasty enough bug that it never hurts to get some extra eyes on it). Thanks!

@marcelgerber
Copy link
Contributor

@peterflynn @RaymondLim I'm sorry to say, but using the files from above recipe, the bug got even worse.
Latest master (03ab969) on Win 8.1

  1. Create the two files i.html and i.css, with the contents:
<html>
    <head>
        <link rel="stylesheet" href="i.css" type="text/css">
    </head>
    <body>
        Some text.
    </body>
</html>
/* body {
    background-color: black;
    color: white;
} */
  1. Start live preview on i.html and wait
  2. Switch to i.css
  3. Click after */

Result: The editor becomes unresponsive

@peterflynn
Copy link
Member

Ugh, sorry -- my fault. My last suggestion for simplifying the loop was not quite right. The stopping condition needs to exactly match how TokenUtils decides whether to stop & return false or not.

@RaymondLim thee potential ways to fix that:

  1. Just change the loop to match movePrevToken() properly -- i.e. while (!(ctx.pos.line <= 0 && (ctx.pos.ch <= 0 || ctx.token.start <= 0)))
  2. Same, but wrap that up in a TokenUtils.isAtStart() utility for clarity
  3. Change movePrevToken() to more consistently stop at the start of the stream, instead of stopping one token sooner when it knows that's the last token available. (This seems more maintainable in the long run, but a tad riskier because it would affect other callers... albeit callers that are probably well unit-tested).

@peterflynn
Copy link
Member

@RaymondLim I can post a fix using approach 2 above -- just proofed it out.

@peterflynn peterflynn mentioned this issue Sep 25, 2014
Merged
@marcelgerber
Copy link
Contributor

Looks good now.

@JeffryBooher
Copy link
Contributor

Closing as fixed. @liaujianjie let us know if you still see problems.

@Mark-Simulacrum Mark-Simulacrum mentioned this issue Oct 26, 2014
Closed
@core-ai-bot core-ai-bot mentioned this issue Aug 30, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@peterflynn peterflynn

Labels
fixed but not closed high priority needs unit test
Projects
None yet
Milestone
Release 0.44
Development

No branches or pull requests
7 participants
@peterflynn @JeffryBooher @RaymondLim @pthiess @marcelgerber @liaujianjie @Mark-Simulacrum