On Wed Apr 3, 2024 at 11:10 AM CEST, Friedrich Weber wrote: > Currently, `Proxmox.window.Edit` initializes `extraRequestParams` to > an object that, if not overwritten, is shared between all instances of > subclasses. This bears the danger of modifying the shared object in a > subclass instead of overwriting it, which affects all edit windows of > the current session and can cause hard-to-catch UI bugs [1]. > > To avoid such bugs in the future, initialize `extraRequestParams` to > `undefined` instead, which forces subclasses to initialize their own > objects. > > Note that bugs of the same kind can still happen if a subclass > initializes `extraRequestParams` to an empty shared object and > inadvertently modifies it, but at least they will be limited to that > particular subclass. > > [1] https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html > > Signed-off-by: Friedrich Weber <f.we...@proxmox.com> > --- > > Notes: > With patch 2/3 applied, I think all occurrences of > `extraRequestParams` in PVE/PBS create their own object (PMG does not > seem to use `extraRequestParams`), so this patch should not break > anything, and if it does, it should be quite noticeable. > > new in v2 > > src/window/Edit.js | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/src/window/Edit.js b/src/window/Edit.js > index d4a2b551..27cd8d01 100644 > --- a/src/window/Edit.js > +++ b/src/window/Edit.js > @@ -9,7 +9,7 @@ Ext.define('Proxmox.window.Edit', { > > // to submit extra params on load and submit, useful, e.g., if not all ID > // parameters are included in the URL > - extraRequestParams: {}, > + extraRequestParams: undefined, > > resizable: false, > > @@ -80,7 +80,9 @@ Ext.define('Proxmox.window.Edit', { > let me = this; > > let values = {}; > - Ext.apply(values, me.extraRequestParams); > + if (me.extraRequestParams) { > + Ext.apply(values, me.extraRequestParams); > + } > > let form = me.formPanel.getForm(); > > @@ -209,7 +211,7 @@ Ext.define('Proxmox.window.Edit', { > waitMsgTarget: me, > }, options); > > - if (Object.keys(me.extraRequestParams).length > 0) { > + if (me.extraRequestParams && Object.keys(me.extraRequestParams).length > > 0) { > let params = newopts.params || {}; > Ext.applyIf(params, me.extraRequestParams); > newopts.params = params;
i did a quick an dirty test and using a constructor like this seems to rule out this class of bug completelly: ```js constructor: function(conf) { let me = this; me.extraRequestParams = {}; me.initConfig(conf); me.callParent(); }, ``` basically it configures the edit window as usual, but overwrites the `extraRequestParams` object for each instance with a new empty object. so there are no more shared objects :) could you check whether that also fixes the other instances? [1]: https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel