tgui: Improve logging, Fix external links (#53226)

* tgui: Improve logging, Fix external links

* Print suspending on the same line

* Streamline fix chat verb

* Muh https

code/__HELPERS/_logging.dm

@@ -202,18 +202,36 @@
 /proc/log_mapping(text)
 	WRITE_LOG(GLOB.world_map_error_log, text)
 
-/* ui logging */
-/proc/log_tgui(user_or_client, text)
+/**
+ * Appends a tgui-related log entry. All arguments are optional.
+ */
+/proc/log_tgui(user, message, context,
+		datum/tgui_window/window,
+		datum/src_object)
 	var/entry = ""
-	if(!user_or_client)
-		entry += "no user"
-	else if(istype(user_or_client, /mob))
-		var/mob/user = user_or_client
-		entry += "[user.ckey] (as [user])"
-	else if(istype(user_or_client, /client))
-		var/client/client = user_or_client
+	// Insert user info
+	if(!user)
+		entry += "<nobody>"
+	else if(istype(user, /mob))
+		var/mob/mob = user
+		entry += "[mob.ckey] (as [mob] at [mob.x],[mob.y],[mob.z])"
+	else if(istype(user, /client))
+		var/client/client = user
 		entry += "[client.ckey]"
-	entry += ":\n[text]"
+	// Insert context
+	if(context)
+		entry += " in [context]"
+	else if(window)
+		entry += " in [window.id]"
+	// Resolve src_object
+	if(!src_object && window && window.locked_by)
+		src_object = window.locked_by.src_object
+	// Insert src_object info
+	if(src_object)
+		entry += "\nUsing: [src_object.type] [REF(src_object)]"
+	// Insert message
+	if(message)
+		entry += "\n[message]"
 	WRITE_LOG(GLOB.tgui_log, entry)
 
 /* For logging round startup. */

code/controllers/subsystem/tgui.dm

@@ -81,7 +81,8 @@ SUBSYSTEM_DEF(tgui)
 			window_found = TRUE
 			break
 	if(!window_found)
-		log_tgui(user, "Error: Pool exhausted")
+		log_tgui(user, "Error: Pool exhausted",
+			context = "SStgui/request_pooled_window")
 		return null
 	return window
 
@@ -93,7 +94,7 @@ SUBSYSTEM_DEF(tgui)
  * required user mob
  */
 /datum/controller/subsystem/tgui/proc/force_close_all_windows(mob/user)
-	log_tgui(user, "force_close_all_windows")
+	log_tgui(user, context = "SStgui/force_close_all_windows")
 	if(user.client)
 		user.client.tgui_windows = list()
 		for(var/i in 1 to TGUI_WINDOW_HARD_LIMIT)
@@ -109,7 +110,7 @@ SUBSYSTEM_DEF(tgui)
  * required window_id string
  */
 /datum/controller/subsystem/tgui/proc/force_close_window(mob/user, window_id)
-	log_tgui(user, "force_close_window")
+	log_tgui(user, context = "SStgui/force_close_window")
 	// Close all tgui datums based on window_id.
 	for(var/datum/tgui/ui in user.tgui_open_uis)
 		if(ui.window && ui.window.id == window_id)

code/modules/client/client_procs.dm

@@ -76,8 +76,11 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
 			to_chat(src, "<span class='danger'>Your previous action was ignored because you've done too many in a second</span>")
 			return
 
-	//Logs all hrefs, except chat pings
-	if(!(href_list["_src_"] == "chat" && href_list["proc"] == "ping" && LAZYLEN(href_list) == 2))
+	// Tgui Topic middleware
+	if(tgui_Topic(href_list))
+		return
+
+	// Log all hrefs
 	log_href("[src] (usr:[usr]\[[COORD(usr)]\]) : [hsrc ? "[hsrc] " : ""][href]")
 
 	//byond bug ID:2256651
@@ -89,10 +92,6 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
 		asset_cache_preload_data(href_list["asset_cache_preload_data"])
 		return
 
-	// Tgui Topic middleware
-	if(tgui_Topic(href_list))
-		return
-
 	// Admin PM
 	if(href_list["priv_msg"])
 		cmd_admin_pm(href_list["priv_msg"],null)

code/modules/tgui/external.dm

@@ -175,7 +175,11 @@
 	var/type = href_list["type"]
 	// Unconditionally collect tgui logs
 	if(type == "log")
-		log_tgui(usr, href_list["message"])
+		var/context = href_list["window_id"]
+		if (href_list["ns"])
+			context += " ([href_list["ns"]])"
+		log_tgui(usr, href_list["message"],
+			context = context)
 	// Reload all tgui windows
 	if(type == "cacheReloaded")
 		if(!check_rights(R_ADMIN) || usr.client.tgui_cache_reloaded)
@@ -195,7 +199,9 @@
 	if(window_id)
 		window = usr.client.tgui_windows[window_id]
 		if(!window)
-			log_tgui(usr, "Error: Couldn't find the window datum, force closing.")
+			log_tgui(usr,
+				"Error: Couldn't find the window datum, force closing.",
+				context = window_id)
 			SStgui.force_close_window(usr, window_id)
 			return TRUE
 	// Decode payload

code/modules/tgui/tgui.dm

@@ -49,7 +49,9 @@
  * return datum/tgui The requested UI.
  */
 /datum/tgui/New(mob/user, datum/src_object, interface, title, ui_x, ui_y)
-	log_tgui(user, "new [interface] fancy [user.client.prefs.tgui_fancy]")
+	log_tgui(user,
+		"new [interface] fancy [user.client.prefs.tgui_fancy]",
+		src_object = src_object)
 	src.user = user
 	src.src_object = src_object
 	src.window_key = "[REF(src_object)]-main"
@@ -245,11 +247,9 @@
 		return
 	// Validate ping
 	if(!initialized && world.time - opened_at > TGUI_PING_TIMEOUT)
-		log_tgui(user, \
-			"Error: Zombie window detected, killing it with fire.\n" \
-			+ "window_id: [window.id]\n" \
-			+ "opened_at: [opened_at]\n" \
-			+ "world.time: [world.time]")
+		log_tgui(user, "Error: Zombie window detected, closing.",
+			window = window,
+			src_object = src_object)
 		close(can_be_suspended = FALSE)
 		return
 	// Update through a normal call to ui_interact
@@ -282,8 +282,12 @@
 /datum/tgui/proc/on_message(type, list/payload, list/href_list)
 	// Pass act type messages to ui_act
 	if(type && copytext(type, 1, 5) == "act/")
+		var/act_type = copytext(type, 5)
+		log_tgui(user, "Action: [act_type] [href_list["payload"]]",
+			window = window,
+			src_object = src_object)
 		process_status()
-		if(src_object.ui_act(copytext(type, 5), payload, src, state))
+		if(src_object.ui_act(act_type, payload, src, state))
 			SStgui.update_uis(src_object)
 		return FALSE
 	switch(type)

code/modules/tgui/tgui_window.dm

@@ -52,7 +52,9 @@
 		inline_assets = list(),
 		inline_html = "",
 		fancy = FALSE)
-	log_tgui(client, "[id]/initialize")
+	log_tgui(client,
+		context = "[id]/initialize",
+		window = src)
 	if(!client)
 		return
 	src.inline_assets = inline_assets
@@ -177,11 +179,15 @@
 	if(!client)
 		return
 	if(can_be_suspended && can_be_suspended())
-		log_tgui(client, "[id]/close: suspending")
+		log_tgui(client,
+			context = "[id]/close (suspending)",
+			window = src)
 		status = TGUI_WINDOW_READY
 		send_message("suspend")
 		return
-	log_tgui(client, "[id]/close")
+	log_tgui(client,
+		context = "[id]/close",
+		window = src)
 	release_lock()
 	status = TGUI_WINDOW_CLOSED
 	message_queue = null

code/modules/tgui_panel/external.dm

@@ -8,46 +8,40 @@
 /**
  * tgui panel / chat troubleshooting verb
  */
-/client/verb/fix_chat()
+/client/verb/fix_tgui_panel()
 	set name = "Fix chat"
 	set category = "OOC"
 	var/action
-	log_tgui(src, "tgui_panel: Started fixing.")
-	// Not initialized
-	if(!tgui_panel || !istype(tgui_panel))
-		log_tgui(src, "tgui_panel: datum is missing")
-		action = alert(src, "tgui panel was not initialized!\nSet it up again?", "", "OK", "Cancel")
-		if(action != "OK")
-			return
-		tgui_panel = new(src)
-		tgui_panel.initialize()
-		action = alert(src, "Wait a bit and tell me if it's fixed", "", "Fixed", "Nope")
-		if(action == "Fixed")
-			log_tgui(src, "tgui_panel: Fixed by calling 'new' + 'initialize'")
-			return
+	log_tgui(src, "Started fixing.",
+		context = "verb/fix_tgui_panel")
 	// Not ready
 	if(!tgui_panel?.is_ready())
-		log_tgui(src, "tgui_panel: not ready")
-		action = alert(src, "tgui panel looks like it's waiting for something.\nSend it a ping?", "", "OK", "Cancel")
-		if(action != "OK")
-			return
+		log_tgui(src, "Panel is not ready",
+			context = "verb/fix_tgui_panel")
 		tgui_panel.window.send_message("ping", force = TRUE)
-		action = alert(src, "Wait a bit and tell me if it's fixed", "", "Fixed", "Nope")
+		action = alert(src, "Method: Pinging the panel.\nWait a bit and tell me if it's fixed", "", "Fixed", "Nope")
 		if(action == "Fixed")
-			log_tgui(src, "tgui_panel: Fixed by sending a ping")
+			log_tgui(src, "Fixed by sending a ping",
+				context = "verb/fix_tgui_panel")
 			return
 	// Catch all solution
-	action = alert(src, "Looks like tgui panel was already setup, but we can always try again.\nSet it up again?", "", "OK", "Cancel")
-	if(action != "OK")
-		return
+	if(!tgui_panel || !istype(tgui_panel))
+		log_tgui(src, "tgui_panel datum is missing",
+			context = "verb/fix_tgui_panel")
+		tgui_panel = new(src)
 	tgui_panel.initialize(force = TRUE)
-	action = alert(src, "Wait a bit and tell me if it's fixed", "", "Fixed", "Nope")
+	// Force show the panel to see if there are any errors
+	winset(src, "output", "is-disabled=1&is-visible=0")
+	winset(src, "browseroutput", "is-disabled=0;is-visible=1")
+	action = alert(src, "Method: Reinitializing the panel.\nWait a bit and tell me if it's fixed", "", "Fixed", "Nope")
 	if(action == "Fixed")
-		log_tgui(src, "tgui_panel: Fixed by calling 'initialize'")
+		log_tgui(src, "Fixed by calling 'initialize'",
+			context = "verb/fix_tgui_panel")
 		return
 	// Failed to fix
 	action = alert(src, "Welp, I'm all out of ideas. Try closing BYOND and reconnecting.\nWe could also disable tgui_panel and re-enable the old UI", "", "Thanks anyways", "Switch to old UI")
 	if (action == "Switch to old UI")
 		winset(src, "output", "on-show=&is-disabled=0&is-visible=1")
 		winset(src, "browseroutput", "is-disabled=1;is-visible=0")
-	log_tgui(src, "tgui_panel: Failed to fix.")
+	log_tgui(src, "Failed to fix.",
+		context = "verb/fix_tgui_panel")

tgui/packages/tgui/links.js

@@ -8,31 +8,33 @@
  * Prevents baby jailing the user when he clicks an external link.
  */
 export const captureExternalLinks = () => {
-  // Click handler
-  const listenerFn = e => {
+  // Subscribe to all document clicks
+  document.addEventListener('click', e => {
     const tagName = String(e.target.tagName).toLowerCase();
-    const href = String(e.target.href);
+    const hrefAttr = e.target.getAttribute('href') || '';
     // Must be a link
     if (tagName !== 'a') {
       return;
     }
     // Leave BYOND links alone
-    const isByondLink = href.charAt(0) === '?'
-      || href.startsWith(location.origin)
-      || href.startsWith('byond://');
+    const isByondLink = hrefAttr.charAt(0) === '?'
+      || hrefAttr.startsWith('byond://');
     if (isByondLink) {
       return;
     }
     // Prevent default action
     e.preventDefault();
+    // Normalize the URL
+    let url = hrefAttr;
+    if (url.toLowerCase().startsWith('www')) {
+      url = 'https://' + url;
+    }
     // Open the link
     Byond.topic({
       tgui: 1,
       window_id: window.__windowId__,
       type: 'openLink',
-      url: href,
+      url,
+    });
   });
-  };
-  // Subscribe to all document clicks
-  document.addEventListener('click', listenerFn);
 };

tgui/packages/tgui/logging.js

@@ -36,6 +36,7 @@ const log = (level, ns, ...args) => {
       tgui: 1,
       window_id: window.__windowId__,
       type: 'log',
+      ns,
       message: logEntry,
     });
   }