Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
jmuehlner merged PR #474: URL: https://github.com/apache/guacamole-server/pull/474 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
necouchman commented on code in PR #474: URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1617997230 ## src/protocols/rdp/channels/rail.h: ## @@ -20,7 +20,34 @@ #ifndef GUAC_RDP_CHANNELS_RAIL_H #define GUAC_RDP_CHANNELS_RAIL_H +#include "config.h" + #include +#include + +#ifdef FREERDP_RAIL_CALLBACKS_REQUIRE_CONST +/** + * FreeRDP 2.0.0-rc4 and newer requires the final argument for all RAIL Review Comment: Well, it depends on how many arguments the functions have. To your point, the `WindowUpdate` callback has two arguments, both of which need to be `const`; however, the `ServerHandshake` and `ServerHandshakeEx` callbacks only have a single argument that is `const`. I've tweaked the comment via rebase. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
jmuehlner commented on code in PR #474: URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1617876716 ## src/protocols/rdp/channels/rail.h: ## @@ -20,7 +20,34 @@ #ifndef GUAC_RDP_CHANNELS_RAIL_H #define GUAC_RDP_CHANNELS_RAIL_H +#include "config.h" + #include +#include + +#ifdef FREERDP_RAIL_CALLBACKS_REQUIRE_CONST +/** + * FreeRDP 2.0.0-rc4 and newer requires the final argument for all RAIL Review Comment: Isn't it the last 2 arguments? That's what I see in the actual function signature. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
necouchman commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1614189805
##
src/protocols/rdp/channels/rail.c:
##
@@ -86,6 +74,7 @@ static UINT
guac_rdp_rail_complete_handshake(RailClientContext* rail) {
};
/* Send client handshake response */
+guac_client_log(client, GUAC_LOG_DEBUG, "Sending RAIL handshake.");
Review Comment:
Fixed via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
necouchman commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1614185189
##
src/protocols/rdp/channels/rail.c:
##
@@ -86,6 +74,7 @@ static UINT
guac_rdp_rail_complete_handshake(RailClientContext* rail) {
};
/* Send client handshake response */
+guac_client_log(client, GUAC_LOG_DEBUG, "Sending RAIL handshake.");
Review Comment:
Yeah, probably so...I'll switch them over to `TRACE`, instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
jmuehlner commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1614093029
##
src/protocols/rdp/channels/rail.c:
##
@@ -86,6 +74,7 @@ static UINT
guac_rdp_rail_complete_handshake(RailClientContext* rail) {
};
/* Send client handshake response */
+guac_client_log(client, GUAC_LOG_DEBUG, "Sending RAIL handshake.");
Review Comment:
The DEBUG-level messages in this file seem more TRACEy than DEBUGy, no?
Having these identical DEBUG messages appear in the log every single time
the handshake is completed feels like overkill to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
necouchman commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1614070701
##
src/protocols/rdp/channels/rail.c:
##
@@ -81,11 +69,12 @@ static UINT
guac_rdp_rail_complete_handshake(RailClientContext* rail) {
/* Build number 7600 (0x1DB0) apparently represents Windows 7 and
* compatibility with RDP 7.0. As of this writing, this is the same
* build number sent for RAIL connections by xfreerdp. */
-.buildNumber = 7600
+.buildNumber = 9600
Review Comment:
Reverted the build number to `7600` - looks like even the current xfreerdp
code still uses 7600 (and sometimes 7601), so we'll just stick with that. I do
not believe it impacts functionality at all and isn't really required for this
change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
necouchman commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1614070118
##
src/protocols/rdp/channels/rail.c:
##
@@ -213,9 +208,63 @@ static UINT guac_rdp_rail_handshake(RailClientContext*
rail,
*/
static UINT guac_rdp_rail_handshake_ex(RailClientContext* rail,
RAIL_CONST RAIL_HANDSHAKE_EX_ORDER* handshake_ex) {
+guac_client* client = (guac_client*) rail->custom;
+guac_client_log(client, GUAC_LOG_TRACE, "RAIL handshake ex callback.");
return guac_rdp_rail_complete_handshake(rail);
}
+/**
+ * A callback function that is executed when an update for a RAIL window is
+ * received from the RDP server.
+ *
+ * @param context
+ * A pointer to the rdpContext structure used by FreeRDP to handle the
+ * window update.
+ *
+ * @param orderInfo
+ * A pointer to the data structure that contains information about what
+ * window was updated what updates were performed.
+ *
+ * @param windowState
+ * A pointer to the data structure that contains details of the updates
+ * to the window, as indicated by flags in the orderInfo field.
+ *
+ * @return
+ * TRUE if the client-side processing of the updates as successful;
otherwise
Review Comment:
Clarified via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
jmuehlner commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1612341552
##
src/protocols/rdp/channels/rail.c:
##
@@ -213,9 +208,63 @@ static UINT guac_rdp_rail_handshake(RailClientContext*
rail,
*/
static UINT guac_rdp_rail_handshake_ex(RailClientContext* rail,
RAIL_CONST RAIL_HANDSHAKE_EX_ORDER* handshake_ex) {
+guac_client* client = (guac_client*) rail->custom;
+guac_client_log(client, GUAC_LOG_TRACE, "RAIL handshake ex callback.");
return guac_rdp_rail_complete_handshake(rail);
}
+/**
+ * A callback function that is executed when an update for a RAIL window is
+ * received from the RDP server.
+ *
+ * @param context
+ * A pointer to the rdpContext structure used by FreeRDP to handle the
+ * window update.
+ *
+ * @param orderInfo
+ * A pointer to the data structure that contains information about what
+ * window was updated what updates were performed.
+ *
+ * @param windowState
+ * A pointer to the data structure that contains details of the updates
+ * to the window, as indicated by flags in the orderInfo field.
+ *
+ * @return
+ * TRUE if the client-side processing of the updates as successful;
otherwise
Review Comment:
Looks to me like it always returns true,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
jmuehlner commented on code in PR #474:
URL: https://github.com/apache/guacamole-server/pull/474#discussion_r1612340336
##
src/protocols/rdp/channels/rail.c:
##
@@ -81,11 +69,12 @@ static UINT
guac_rdp_rail_complete_handshake(RailClientContext* rail) {
/* Build number 7600 (0x1DB0) apparently represents Windows 7 and
* compatibility with RDP 7.0. As of this writing, this is the same
* build number sent for RAIL connections by xfreerdp. */
-.buildNumber = 7600
+.buildNumber = 9600
Review Comment:
This comment is now out of date.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GUACAMOLE-1231: Implement basic support for restoring minimized RAIL windows [guacamole-server]
mike-jumper commented on PR #474: URL: https://github.com/apache/guacamole-server/pull/474#issuecomment-1882641180 > This is a really basic update for the Guacamole RAIL channel that handles situations where RAIL windows are minimized by simply immediately sending the `SC_RESTORE` command for that window back to the RDP server, causing the window to immediately re-appear. ... I rather like this. It immediately mitigates the issue at hand without requiring huge changes, but also doesn't get in the way of some future representation for minimized windows. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
