Xpra-org / xpra-html5

HTML5 client for Xpra
Mozilla Public License 2.0
192 stars 53 forks source link

Bugfix URL parameters not being applied #292

Closed EmanH closed 4 months ago

EmanH commented 4 months ago

URL parameters are not being applied in connect.html or index.html. The root cause is that the getparam() function in Utitiles is effectively ignoring url parameters.

totaam commented 4 months ago

Parameters should override session storage, by changing the order, this doesn't work any more. Does it?

EmanH commented 4 months ago

@totaam Yes, it doesn't work. My assumption was that session params take precedence with URL params as the fallback for any undefined value. Either way makes no difference to me though, so long as URL params are being applied.

totaam commented 4 months ago

Please keep the parameters as overrides for sessionStorage

EmanH commented 4 months ago

@totaam Have changed so url parameters take precedence over parameters from sessionStorage

totaam commented 4 months ago

I have made a mistake in merging this too quickly.

I have some fixes pending.

totaam commented 4 months ago

@EmanH Please review the commit above. I have reviewed all the other call sites - it doesn't look like they require changes.

EmanH commented 4 months ago

@totaam Technically == does a fuzzy compare (in JS), so in effect:

So, these are equivalent:

if (v == null && prop in default_settings) {}
if (v == undefined && prop in default_settings) {}

The other change from if (v === null) { to if (!v) {. Is probably ok, but also fuzzy compare. So !v would pass if v was undefined, null, false or '' (empty string).

EmanH commented 4 months ago

When assigning values, I think its best practice to consider undefined as 'no value' and consider null as 'a value'.

totaam commented 4 months ago

@EmanH I know the difference. The ones in stristrue and getboolparam were definitely bugs caused by this change as if (v === null) would no longer match. The other in getparam was changed for consistency. Using !v is just a shortcut, avoids converting null, undefined, empty-strings, False-booleans, etc.

totaam commented 3 months ago

And... another fix, these changes completely broke the connect page.