Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create RFB object on connect #473

Closed
wants to merge 1 commit into from

Conversation

DirectXMan12
Copy link
Member

In e543525, we switched to creating
a new RFB object on disconnect. This caused issues, however, since
any errors were only displayed briefly before the new "loaded" text
was displayed instead.

Now, we create the RFB object on connect. This essentially removes
the usefulness of the "loaded" state, but prevents the aforementioned
problem.

To facilitate this, the code which does detection of cursor URI support
was moved from this Display constructor (which now calls the new
function) into its own function, Util.browserSupportsCursorURIs().

Fixes #467

@samhed
Copy link
Member

samhed commented Mar 25, 2015

Sweet! Looks like a clean solution 👍

On #467 you wrote:

and the status bar defaults to having a class of noVNC_status_normal with some initial boilerplate text

With these changes, the statusbar is empty initially if I'm not mistaken. Maybe it should be empty? I will test this tomorrow and decide what I think.

@DirectXMan12
Copy link
Member Author

With these changes, the statusbar is empty initially if I'm not mistaken

Correct. I couldn't quite decide what to put in the status bar, so I left it empty ;-)

@samhed
Copy link
Member

samhed commented Mar 25, 2015

so I left it empty ;-)

I think it looks good like that!

However, explicitly specifying the class of the control bar in the HTML conflict with the way we set the style in updateState in ui.js. With this patch the class is never changed, i.e we don't get the warning/error gradient colors.

In e543525, we switched to creating
a new RFB object on disconnect.  This caused issues, however, since
any errors were only displayed briefly before the new "loaded" text
was displayed instead.

Now, we create the RFB object on connect.  This essentially removes
the usefulness of the "loaded" state, but prevents the aforementioned
problem.

To facilitate this, the code which does detection of cursor URI support
was moved from this Display constructor (which now calls the new
function) into its own function, `Util.browserSupportsCursorURIs()`.

Fixes #467
@DirectXMan12
Copy link
Member Author

@samhed : I just set the class on the wrong div ;-) It should work fine now.

@samhed
Copy link
Member

samhed commented Mar 27, 2015

God I must've been tired when writing that! It does indeed work fine now.

Let's close this in favor for #474 which includes these changes then.

@samhed samhed closed this Mar 27, 2015
@DirectXMan12 DirectXMan12 deleted the bug/auth-errors-not-displayed branch March 27, 2015 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentication errors are not displayed
2 participants