From 821906971cfd50aba25f95fa5c75b8b1d387edd7 Mon Sep 17 00:00:00 2001
From: Jonathan Wilkes <jon.w.wilkes@gmail.com>
Date: Mon, 19 Dec 2016 20:04:26 -0500
Subject: [PATCH] fix #194: saving a loaded abstraction gives errors. Cleaned
up dangling reference to closed nw window, and protect against out-of-order
messages that come before the canvas is mapped
---
pd/nw/index.js | 21 ++++++++++++++++++++-
pd/nw/pdgui.js | 45 ++++++++++++++++++++++++++++++---------------
2 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/pd/nw/index.js b/pd/nw/index.js
index d734d47f3..2436ef49c 100644
--- a/pd/nw/index.js
+++ b/pd/nw/index.js
@@ -374,7 +374,6 @@ function nw_close_window(window) {
window.close(true);
}
-// Way too many arguments here-- rethink this interface
function nw_create_window(cid, type, width, height, xpos, ypos, attr_array) {
// todo: make a separate way to format the title for OSX
var my_title;
@@ -412,6 +411,7 @@ function nw_create_window(cid, type, width, height, xpos, ypos, attr_array) {
pdgui.set_dialogwin(cid, new_win);
}
new_win.on("loaded", function() {
+ // Off to the races... :(
// We need to check here if we're still the window pointed to
// by the GUI's array of toplevel windows. It can easily happen
// that we're not-- for example the user could send a stream
@@ -424,12 +424,31 @@ function nw_create_window(cid, type, width, height, xpos, ypos, attr_array) {
// as a loaded canvas. If not, we assume it got closed before
// we were able to finish loading the browser window (e.g.,
// with a [vis 1, vis 0( message). In that case we kill the window.
+
+ // Still, this is pretty fortuitous-- we have two levels of
+ // asynchronicity-- creating the nw window and loading it. There
+ // may still be an edge case where a race between those two causes
+ // unpredictable behavior.
if ((new_win === pdgui.get_patchwin(cid) ||
new_win === pdgui.get_dialogwin(cid))
&& pdgui.window_is_loading(cid)) {
// initialize the window
new_win.eval(null, eval_string);
+ // flag the window as loaded. We may want to wait until the
+ // DOM window has finished loading for this.
+ pdgui.set_finished_loading(cid);
} else {
+ // If the window is no longer loading, we need to go ahead
+ // and remove the reference to it in the patchwin array.
+ // Otherwise we get dangling references to closed windows
+ // and other bugs...
+ if (!pdgui.window_is_loading(cid)) {
+ if (type === "pd_canvas") {
+ pdgui.set_patchwin(cid, undefined);
+ } else {
+ pdgui.set_dialogwin(cid, undefined);
+ }
+ }
new_win.close(true);
}
});
diff --git a/pd/nw/pdgui.js b/pd/nw/pdgui.js
index e92043279..b318f1045 100644
--- a/pd/nw/pdgui.js
+++ b/pd/nw/pdgui.js
@@ -1247,6 +1247,12 @@ function window_is_loading(cid) {
exports.window_is_loading = window_is_loading;
+function set_window_finished_loading(cid) {
+ loading[cid] = null;
+}
+
+exports.set_window_finished_loading = set_window_finished_loading;
+
// wrapper for nw_create_window
function create_window(cid, type, width, height, xpos, ypos, attr_array) {
nw_create_window(cid, type, width, height, xpos, ypos, attr_array);
@@ -1924,11 +1930,14 @@ function gui_canvas_deselect_line(cid, tag) {
// rename to erase_line (or at least standardize with gobj_erase)
function gui_canvas_delete_line(cid, tag) {
- var line = get_item(cid, tag);
- if (line !== null) {
- line.parentNode.removeChild(line);
- } else {
- //post("canvas_delete_line: error: the line doesn't exist");
+ var line;
+ if (patchwin[cid]) {
+ line = get_item(cid, tag);
+ if (line !== null) {
+ line.parentNode.removeChild(line);
+ } else {
+ //post("canvas_delete_line: error: the line doesn't exist");
+ }
}
}
@@ -2167,11 +2176,14 @@ function gui_gobj_select(cid, tag) {
}
function gui_gobj_deselect(cid, tag) {
- var gobj = get_gobj(cid, tag)
- if (gobj !== null) {
- gobj.classList.remove("selected");
- } else {
- console.log("text_deselect: something wrong with tag: " + tag + "gobj");
+ var gobj;
+ if (patchwin[cid]) {
+ gobj = get_gobj(cid, tag);
+ if (gobj !== null) {
+ gobj.classList.remove("selected");
+ } else {
+ console.log("text_deselect: error with tag: " + tag + "gobj");
+ }
}
}
@@ -2809,11 +2821,14 @@ function gui_iemgui_label_color(cid, tag, color) {
}
function gui_iemgui_label_select(cid, tag, is_selected) {
- var svg_text = get_item(cid, tag + "label");
- if (is_selected) {
- svg_text.classList.add("iemgui_label_selected");
- } else {
- svg_text.classList.remove("iemgui_label_selected");
+ var svg_text;
+ if (patchwin[cid]) {
+ svg_text = get_item(cid, tag + "label");
+ if (is_selected) {
+ svg_text.classList.add("iemgui_label_selected");
+ } else {
+ svg_text.classList.remove("iemgui_label_selected");
+ }
}
}
--
GitLab