Change prefix of auto-generated field names

Fixes behaviour with luanti-org/luanti#14878

I've also added a workaround to preserve the current behaviour of
dropdowns that don't have index_event set.

.luacheckrc

@@ -1,14 +1,15 @@
 max_line_length = 80
 
 globals = {
-    'formspec_ast',
-    'minetest',
-    'hud_fs',
     'flow',
-    'dump',
 }
 
 read_globals = {
+    'dump',
+    'formspec_ast',
+    'fs51',
+    'hud_fs',
+    'minetest',
     string = {fields = {'split', 'trim'}},
     table = {fields = {'copy', 'indexof'}}
 }

README.md

@@ -37,6 +37,8 @@ Vaguely inspired by Flutter and GTK.
  - This mod doesn't support all of the features that regular formspecs do.
  - [FS51](https://content.minetest.net/packages/luk3yx/fs51/) is required if
    you want to have full support for Minetest 5.3 and below.
+ - Make sure you're using the latest version of flow if you are on MT 5.10-dev
+   or later, older versions used a hack which no longer works.
 
 ## Basic example
 
@@ -593,4 +595,6 @@ local parent_form = flow.make_gui(function(player, ctx)
 end)
 ```
 
+Special characters (excluding `-` and `_`) are not allowed in embed names.
+
 </details>

embed.lua

@@ -78,7 +78,7 @@ return function(self, fields)
         return self._build(player, parent_ctx)
     end
 
-    local prefix = "\2" .. name .. "\2"
+    local prefix = "_#" .. name .. "#"
     local child_ctx = embed_create_ctx(parent_ctx, name, prefix)
     change_ctx(child_ctx)
     local root_node = self._build(player, child_ctx)

init.lua

@@ -317,7 +317,7 @@ function align_types.fill(node, x, w, extra_space)
         node[1] = {
             type = "style",
             -- MT 5.1.0 only supports one style selector
-            selectors = {"\1"},
+            selectors = {"_#"},
 
             -- bgimg_pressed is included for 5.1.0 support
             -- bgimg_hovered is unnecessary as it was added in 5.2.0 (which
@@ -329,7 +329,7 @@ function align_types.fill(node, x, w, extra_space)
         -- removed
         node[2] = {
             type = "style",
-            selectors = {"\1:hovered", "\1:pressed"},
+            selectors = {"_#:hovered", "_#:pressed"},
             props = {bgimg = ""},
         }
 
@@ -339,7 +339,7 @@ function align_types.fill(node, x, w, extra_space)
             drawborder = false,
             x = 0, y = 0,
             w = node.w + extra_space, h = node.h,
-            name = "\1", label = node.label,
+            name = "_#", label = node.label,
             style = node.style,
         }
 
@@ -350,7 +350,7 @@ function align_types.fill(node, x, w, extra_space)
             drawborder = false,
             x = 0, y = 0,
             w = node.w + extra_space, h = node.h,
-            name = "\1", label = "",
+            name = "_#", label = "",
         }
 
         node.y = node.y - LABEL_OFFSET
@@ -639,12 +639,32 @@ function field_value_transformers.tabheader(node)
     return range_check_transformer(node.captions and #node.captions or 0)
 end
 
-function field_value_transformers.dropdown(node)
+function field_value_transformers.dropdown(node, _, formspec_version)
     local items = node.items or {}
-    if node.index_event then
+    if node.index_event and not node._index_event_hack then
         return range_check_transformer(#items)
     end
 
+    -- MT will start sanitising formspec fields on its own at some point
+    -- (https://github.com/minetest/minetest/pull/14878), however it may strip
+    -- escape sequences from dropdowns as well. Since we know what the actual
+    -- value of the dropdown is anyway, we can just enable index_event for new
+    -- clients and keep the same behaviour
+    if (formspec_version and formspec_version >= 4) or
+            (minetest.global_exists("fs51") and fs51.monkey_patching_enabled) then
+        node.index_event = true
+
+        -- Detect reuse of the same Dropdown element (this is unsupported and
+        -- will break in other ways)
+        node._index_event_hack = true
+
+        return function(value)
+            return items[tonumber(value)]
+        end
+    elseif node._index_event_hack then
+        node.index_event = nil
+    end
+
     -- Make sure that the value sent by the client is in the list of items
     return function(value)
         if table.indexof(items, value) > 0 then
@@ -708,7 +728,7 @@ local button_types = {
 
 -- Removes on_event from a formspec_ast tree and returns a callbacks table
 local function parse_callbacks(tree, ctx_form, auto_name_id,
-        replace_backgrounds)
+        replace_backgrounds, formspec_version)
     local callbacks
     local btn_callbacks = {}
     local saved_fields = {}
@@ -809,7 +829,8 @@ local function parse_callbacks(tree, ctx_form, auto_name_id,
 
                 local get_transformer = field_value_transformers[node.type]
                 saved_fields[node_name] = get_transformer and
-                    get_transformer(node, tablecolumn_count) or
+                    get_transformer(node, tablecolumn_count,
+                        formspec_version) or
                     default_field_value_transformer
             end
         end
@@ -818,7 +839,9 @@ local function parse_callbacks(tree, ctx_form, auto_name_id,
         if node.on_event then
             local is_btn = button_types[node.type]
             if not node_name then
-                node_name = ("\1%x"):format(auto_name_id)
+                -- Flow internal field names start with "_#" to avoid
+                -- conflicts with user-provided fields.
+                node_name = ("_#%x"):format(auto_name_id)
                 node.name = node_name
                 auto_name_id = auto_name_id + 1
             elseif btn_callbacks[node_name] or
@@ -1043,7 +1066,7 @@ function Form:_render(player, ctx, formspec_version, id1, embedded, lang_code)
 
     local tree = render_ast(box, embedded)
     local callbacks, btn_callbacks, saved_fields, id2 = parse_callbacks(
-        tree, orig_form, id1, embedded
+        tree, orig_form, id1, embedded, formspec_version
     )
 
     -- This should be after parse_callbacks so it can take advantage of

test.lua

@@ -81,12 +81,12 @@ string.split = string.split or function(str, chr)
 end
 
 function minetest.explode_textlist_event(event)
-    local event_type, number = event:match("^([A-Z]+):(%d+)$")
+    local event_type, number = event:match("^([A-Z]+):(%d+)#")
     return {type = event_type, index = tonumber(number) or 0}
 end
 
 function minetest.explode_table_event(event)
-    local event_type, row, column = event:match("^([A-Z]+):(%d+):(%d+)$")
+    local event_type, row, column = event:match("^([A-Z]+):(%d+):(%d+)#")
     return {type = event_type, row = tonumber(row) or 0, column = tonumber(column) or 0}
 end
 
@@ -352,7 +352,7 @@ describe("Flow", function()
 
             gui.List{inventory_location = "a", list_name = "b", w = 2, h = 2},
             gui.Style{selectors = {"test"}, props = {prop = "value"}},
-        }, ([[
+        }, [[
             size[3.6,3.6]
             button[0.3,0.3;3,1;;1]
             image[0.3,0.3;3,3;2]
@@ -361,14 +361,14 @@ describe("Flow", function()
             field_close_on_enter[5;false]
             field[0.3,0.3;3,3;5;;]
 
-            style[\1;bgimg=;bgimg_pressed=]
-            style[\1:hovered,\1:pressed;bgimg=]
-            image_button[0.3,1.6;3,0.4;blank.png;\1;Test;;false]
-            image_button[0.3,1.6;3,0.4;blank.png;\1;;;false]
+            style[_#;bgimg=;bgimg_pressed=]
+            style[_#:hovered,_#:pressed;bgimg=]
+            image_button[0.3,1.6;3,0.4;blank.png;_#;Test;;false]
+            image_button[0.3,1.6;3,0.4;blank.png;_#;;;false]
 
             list[a;b;0.675,0.675;2,2]
             style[test;prop=value]
-        ]]):gsub("\\1", "\1"))
+        ]])
     end)
 
     it("ignores gui.Nil", function()
@@ -816,11 +816,11 @@ describe("Flow", function()
             test_render(gui.Button{
                 w = 1, h = 1, on_event = function() end,
                 style = {hello = "world"}
-            }, ([[
+            }, [[
                 size[1.6,1.6]
-                style[\10;hello=world]
+                style[_#0;hello=world]
                 button[0.3,0.3;1,1;\10;]
-            ]]):gsub("\\1", "\1"))
+            ]])
         end)
 
         it("supports advanced selectors", function()
@@ -951,7 +951,7 @@ describe("Flow", function()
                     gui.Label{label = "This is the embedded form!"},
                     -- The exact prefix is an implementation detail, you
                     -- shouldn't rely on this in your own code
-                    gui.Field{name = "\2theprefix\2test2"},
+                    gui.Field{name = "_#theprefix#test2"},
                 },
                 gui.Label{label = "ffaksksdf"}
             })
@@ -997,15 +997,15 @@ describe("Flow", function()
                 gui.Label{label = "asdft"},
                 gui.VBox{
                     gui.Label{label = "This is the embedded form!"},
-                    gui.Field{name = "\2theprefix\2test2"},
+                    gui.Field{name = "_#theprefix#test2"},
                     gui.Label{label = "A is true! WOW!"}
                 },
                 gui.Label{label = "ffaksksdf"}
             })
         end)
         it("flow form context table", function()
             test_render(function(p, x)
-                x.form["\2the_name\2jkl"] = 3
+                x.form["_#the_name#jkl"] = 3
                 local child = flow.make_gui(function(_p, xc)
                     xc.form.thingy = true
                     xc.form.jkl = 9
@@ -1014,8 +1014,8 @@ describe("Flow", function()
                     player = p,
                     name = "the_name"
                 }
-                assert.True(x.form["\2the_name\2thingy"])
-                assert.equal(9, x.form["\2the_name\2jkl"])
+                assert.True(x.form["_#the_name#thingy"])
+                assert.equal(9, x.form["_#the_name#jkl"])
                 return child
             end, gui.Label{label = "asdf"})
         end)
@@ -1026,7 +1026,7 @@ describe("Flow", function()
                 return e
             end, gui.VBox{
                 gui.Label{label = "This is the embedded form!"},
-                gui.Field{name = "\2asdf\2test2"},
+                gui.Field{name = "_#asdf#test2"},
                 gui.Box{w = 1, h = 3}
             })
         end)
@@ -1059,8 +1059,8 @@ describe("Flow", function()
 
             local player, ctx = wrapped_p, state.ctx
             state.callbacks.quit(player, ctx)
-            state.callbacks["\2thesubform\2field"](player, ctx)
-            state.btn_callbacks["\2thesubform\2btn"](player, ctx)
+            state.callbacks["_#thesubform#field"](player, ctx)
+            state.btn_callbacks["_#thesubform#btn"](player, ctx)
 
             assert.same(state.ctx.thesubform, wrapped_x)
 
@@ -1073,8 +1073,8 @@ describe("Flow", function()
 
             -- Each of these are wrapped with another function to put the actual function in the correct environment
             assert.Not.same(func_quit, state.callbacks.quit)
-            assert.Not.same(func_field_event, state.callbacks["\2thesubform\2field"])
-            assert.Not.same(func_btn_event, state.callbacks["\2thesubform\2btn"])
+            assert.Not.same(func_field_event, state.callbacks["_#thesubform#field"])
+            assert.Not.same(func_btn_event, state.callbacks["_#thesubform#btn"])
         end)
         describe("metadata", function()
             it("style data is modified", function()
@@ -1086,7 +1086,7 @@ describe("Flow", function()
                 test_render(function(p, _x)
                     return style_embedded_form:embed{player = p, name = "asdf"}
                 end, gui.VBox{
-                    gui.Style{selectors = {"\2asdf\2test"}, props = {prop = "value"}},
+                    gui.Style{selectors = {"_#asdf#test"}, props = {prop = "value"}},
                 })
             end)
             it("scroll_container data is modified", function()
@@ -1098,7 +1098,7 @@ describe("Flow", function()
                 test_render(function(p, _x)
                     return scroll_embedded_form:embed{player = p, name = "asdf"}
                 end, gui.VBox{
-                    gui.ScrollContainer{scrollbar_name = "\2asdf\2name"}
+                    gui.ScrollContainer{scrollbar_name = "_#asdf#name"}
                 })
             end)
             it("tooltip data is modified", function()
@@ -1110,7 +1110,7 @@ describe("Flow", function()
                 test_render(function(p, _x)
                     return tooltip_embedded_form:embed{player = p, name = "asdf"}
                 end, gui.VBox{
-                    gui.Tooltip{gui_element_name = "\2asdf\2lololol"}
+                    gui.Tooltip{gui_element_name = "_#asdf#lololol"}
                 })
             end)
         end)
@@ -1130,7 +1130,7 @@ describe("Flow", function()
                 assert.same("new value!", x.asdf.field, "values that it set set are here")
                 return subform
             end, gui.VBox{
-                gui.Field{name = "\2asdf\2field"}
+                gui.Field{name = "_#asdf#field"}
             })
         end)
         it("supports fresh initial form values", function()
@@ -1148,7 +1148,7 @@ describe("Flow", function()
                 end
                 return tooltip_embedded_form:embed{player = p, name = "asdf"}
             end, gui.VBox{
-                gui.Field{name = "\2asdf\2field"}
+                gui.Field{name = "_#asdf#field"}
             })
         end)
         it("updates flow.get_context", function()