Xpra: Ticket #2705: js syntax

changes: reduced hints generally

Thats why I found:



Thu, 02 Apr 2020 21:56:56 GMT - brief: attachment set


Thu, 02 Apr 2020 21:59:07 GMT - brief: priority, type, component changed

Should I start a new ticket to discuss linting? The non consistent syntax makes creating patches without withspace changes very hard.


Fri, 03 Apr 2020 04:17:54 GMT - Antoine Martin: owner changed

I don't doubt that some of these changes may well be valid, but I can't just apply them blindly. Can you comment?

I would much prefer removing "id_XXXX":

=== "function" will never match? Same in: else if (CLIPBOARD_IMAGES && dtype=="image/png" && dformat==8 && wire_encoding=="bytes" && navigator.clipboard && navigator.clipboard.write) {

etc.. What sort of problems is this meant to prevent?

Also, please try to keep indentation / semi-colon type of patches separate from the changes that will modify behaviour. We can apply the former very quickly, as it is quick to review, the latter may take more time.


Fri, 03 Apr 2020 04:21:55 GMT - Antoine Martin:

Should I start a new ticket to discuss linting?

I don't mind.

The non consistent syntax makes creating patches without whitespace changes very hard.

We can merge one change that only changes whitespace and nothing else. Then things will be easier?


Fri, 03 Apr 2020 19:59:06 GMT - brief:

Replying to Antoine Martin:

I don't doubt that some of these changes may well be valid, but I can't just apply them blindly. Can you comment?

I would much prefer removing "id_XXXX":

So remove the id_ part of variables? Or dont remove duplicate jQuery calls?

=== "function" will never match?

typeof window.clipboardData.setData === "function" will match

Same in: else if (CLIPBOARD_IMAGES && dtype=="image/png" && dformat==8 && wire_encoding=="bytes" && navigator.clipboard && navigator.clipboard.write) {


clog("file:", f.name, ", type:", f.type, ", size:", f.size, "last modified:", f.hasOwnProperty("lastModifiedDate") ? f.lastModifiedDate.toLocaleDateString() : 'n/a');
if (CLIPBOARD_IMAGES && navigator.clipboard && navigator.clipboard.hasOwnProperty("write")) {
navigator.hasOwnProperty("connection")

etc.. What sort of problems is this meant to prevent?

With clipboardData.setData the attribute is accessed, clipboardData.hasOwnProperty("setData") checks if there is such an attribute (what was meant with clipboardData.setData). On expecting a function, typeof window.clipboardData.setData === "function" needs to be used. It results in no access to undefined objects (and also no warnings).

It is commented out to point out that it is not used, can be deleted.

Yes

ctx is used in the line below like ctx.debug("tray", "tray geometry changed (ignored)");

This is an error. My intention was to only delete the closing img tag (</img>) as this does not exists.

I wanted to get rid of the duplicate jQuery call, so I introduced a variable class_notifications.

window is in used by JS, what about window_element?

Also, please try to keep indentation / semi-colon type of patches separate from the changes that will modify behaviour. We can apply the former very quickly, as it is quick to review, the latter may take more time.


Fri, 03 Apr 2020 21:44:47 GMT - brief: attachment set


Fri, 03 Apr 2020 21:44:53 GMT - brief: attachment set


Fri, 03 Apr 2020 21:44:59 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:06 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:12 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:17 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:23 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:28 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:51 GMT - brief: attachment set


Fri, 03 Apr 2020 21:45:56 GMT - brief: attachment set


Fri, 03 Apr 2020 21:46:02 GMT - brief: attachment set


Fri, 03 Apr 2020 21:46:08 GMT - brief: attachment set


Fri, 03 Apr 2020 21:48:17 GMT - brief:

Like this? These are not incremental, is this even possible? Do you create a separate branch on merging?


Sat, 04 Apr 2020 04:05:00 GMT - Antoine Martin:

Like this?

Perfect, thank you.

Updates:

Not done yet:

Only problem is that something broke and makes the html5 client unusable:

Uncaught ReferenceError: Buffer is not defined
    at Object.<anonymous> (forge.js:25985)
    at __webpack_require__ (forge.js:30)
    at Object.<anonymous> (forge.js:25412)
    at __webpack_require__ (forge.js:30)
    at Object.<anonymous> (forge.js:25391)
    at __webpack_require__ (forge.js:30)
    at forge.js:73
    at forge.js:76
    at webpackUniversalModuleDefinition (forge.js:9)
    at forge.js:10

Sat, 04 Apr 2020 05:41:27 GMT - Antoine Martin:

Uncaught ReferenceError: Buffer is not defined

Fixed in r25953.


Sat, 04 Apr 2020 06:28:45 GMT - Antoine Martin:

Updates:


Sat, 04 Apr 2020 11:24:16 GMT - Antoine Martin:

r25965 fixes uglifyjs using regexp substitutions, so we lose the "const" and "let" changes during minification... This will have to do until we switch to terser (#2709).


Wed, 08 Apr 2020 14:13:27 GMT - Antoine Martin:

@brief: can I close this or do you want to add more?


Thu, 09 Apr 2020 14:27:17 GMT - brief: attachment set


Thu, 09 Apr 2020 14:29:52 GMT - brief:

This should be the last one.

Replying to Antoine Martin:

I only made one patch now since this looks like a line mismatch while merging.


Thu, 09 Apr 2020 14:55:01 GMT - Antoine Martin:

Thanks!

From the "final" patch:

As per comment:8, the check for === "function" does not work under chrome, so I have ignored that. Also, I am not merging the const Buffer and const LZ4 because of the problems this causes, see comment:7 (though the problem may not show since we replace const by var before using the minifier - nonetheless, AFAICT this is still a problem)


Sun, 12 Apr 2020 16:11:19 GMT - Antoine Martin: status changed; resolution set

Closing, let's continue this in a new ticket for 4.1


Sat, 23 Jan 2021 05:58:34 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/2705