GPII-2729: Onboard Windows 10 Internationalization settings #158

JavierJF · 2017-12-08T15:52:06Z

No description provided.

…windows messages

…egistrySettingsHandler

… universal

gpii-bot · 2017-12-08T15:55:21Z

Could one of the admins verify that these changes are reasonable to test? If so, please reply with "ok to test".

kaspermarkus · 2017-12-13T17:55:00Z

@JavierJF will notify @stegru when this is ready for review... He'll give it a first round of review, and pass to antranig when ready for merge/second round of review

stegru · 2017-12-19T22:06:01Z

gpii/node_modules/registrySettingsHandler/src/RegistrySettingsHandler.js

    return togo;
 };

+windows.isSupportedMsg = function(message) {


I like the idea of being able to send a message. However, I think it should be placed into a function and called in a "start" or "stop" block, like gpii.launch.exec or gpii.windows.closeProcessByName.

That way, other settings handlers can make use of it.

I like that thought, will be simpler, and useful from all the SettingsHandlers, sadly we have a imminent change for that block coming from this pull. So probably I'm going to update the rest of issues of the pull and leave this one as a TODO, to be fixed when the new blocks could be used.

stegru · 2017-12-19T22:19:06Z

gpii/node_modules/registrySettingsHandler/src/RegistrySettingsHandler.js

 };

+windows.isSupportedMsg = function(message) {
+    var hwnd = windows.API_constants[message.hwnd];


Why bother? It's not like there can be other values for the hWnd.

I suggest either remove the ability to specify the hWnd, or perhaps accept a string, where it sends the message to windows with a matching caption (and/or classname?), if it's not "HWND_BROADCAST" (using a combination of windows.enumerateWindows and GetWindowText).

stegru · 2017-12-19T22:22:59Z

gpii/node_modules/registrySettingsHandler/src/RegistrySettingsHandler.js

+
+windows.sendWindowMessage = function(message) {
+    try {
+        if (!windows.isSupportedMsg(message)) {


Why not just do !hwnd || !msg on 367

stegru · 2017-12-20T11:14:59Z

@JavierJF, that's my first-pass review. I haven't ran it (it has the "Needs Work" label).

I agree with your approach. There is a SetLocalInfo API function, and I think that does exactly what you've done - I mean, this works, right? It might be worth creating a small .c program that just calls that function to see if it does anything else... but I doubt it does.

I also have an idea - perhaps there could also be a "culture" setting (en-GB, es-ES). It would make all of these individual settings whatever that culture default is for that setting, unless specifically set. GetLocalInfoEx gets this information.

gpii-bot · 2018-02-17T13:26:19Z

Could one of the admins verify that these changes are reasonable to test? If so, please reply with "ok to test".

stegru · 2018-04-13T10:43:21Z

ok to test

gpii-bot · 2018-04-13T11:19:53Z

CI job failed: https://ci.gpii.net/job/windows-tests/461/

stegru · 2018-04-13T11:49:46Z

What changed since I last looked?

My suggestions:

The registry handler isn't the best place to send messages from. There's now a windowMessages module, https://github.com/gpii/windows/blob/master/gpii/node_modules/windowMessages/src/windowMessages.js (it currently just receives messages)
Use the same mechanism as gpii.launch.exec or gpii.windows.closeProcessByName, so other solutions without registry settings can enjoy sending messages.
No need to pass HWND_BROADCAST (just always use it), because there is no other valid window handle constant that I'm aware of. Bonus points: allow passing the window class name.
Maybe put the WM_xx constants in their own object.

gpii-bot · 2018-06-06T11:32:02Z

CI job failed: https://ci.gpii.net/job/windows-tests/521/

+ Removed the ability to specify windows handle in message feature. + Fixed some linter issues.

gpii-bot · 2018-06-08T21:41:11Z

CI job passed: https://ci.gpii.net/job/windows-tests/534/

gpii-bot · 2018-06-11T22:38:22Z

CI job passed: https://ci.gpii.net/job/windows-tests/535/

…jes module

… the RegistrySettingsHandler

gpii-bot · 2018-06-14T20:14:14Z

CI job passed: https://ci.gpii.net/job/windows-tests/538/

gpii-bot · 2018-06-14T20:39:59Z

CI job passed: https://ci.gpii.net/job/windows-tests/539/

gpii-bot · 2018-06-14T21:11:34Z

CI job failed: https://ci.gpii.net/job/windows-tests/540/

gpii-bot · 2018-06-14T21:40:45Z

CI job passed: https://ci.gpii.net/job/windows-tests/541/

stegru · 2018-06-19T10:45:13Z

gpii/node_modules/windowMessages/src/windowMessages.js

+    return (typeof msg !== undefined);
+};
+
+windows.sendWindowMessage = function (message) {


A jsdoc would be nice

stegru · 2018-06-19T10:58:04Z

gpii/node_modules/windowMessages/src/windowMessages.js

+
+windows.sendWindowMessage = function (message) {
+    try {
+        if (!windows.isSupportedMsg(message)) {


I don't see the point in this any more, considering a check on the message constant is performed on line 258. An else block can be used at 261 instead.

Yes, replacing it for a 'else' for having a switch-case style message check, and logging in case of unsupported.

stegru · 2018-06-19T15:50:18Z

gpii/node_modules/windowMessages/test/windowMessagesTest.js

 });
+
+// Test that the "sendWindowMessage" function actually sends the message.
+jqUnit.test("Window Messages - Message sent", function () {


as discussed

gpii-bot · 2018-06-20T12:40:48Z

CI job failed: https://ci.gpii.net/job/windows-tests/542/

stegru · 2018-06-20T14:21:05Z

ok to test

gpii-bot · 2018-06-20T14:50:00Z

CI job failed: https://ci.gpii.net/job/windows-tests/544/

gpii-bot · 2018-06-20T15:47:04Z

CI job failed: https://ci.gpii.net/job/windows-tests/547/

stegru · 2018-06-20T15:47:55Z

ok to test

    default: WinRM transport: negotiate
An error occurred executing a remote WinRM command.

Shell: Cmd
Command: hostname
Message: Unable to parse authorization header. Headers: {}
Body: SSH-2.0-OpenSSH_7.6
Protocol mismatch.
 ().
Build step 'Execute shell' marked build as failure
Finished: FAILURE

gpii-bot · 2018-06-20T16:14:09Z

CI job passed: https://ci.gpii.net/job/windows-tests/548/

gpii-bot · 2018-07-14T18:39:46Z

CI job passed: https://ci.gpii.net/job/windows-tests/578/

stegru · 2018-08-15T14:27:44Z

gpii/node_modules/windowMessages/src/windowMessages.js

+/**
+ * Sends a message to every open window.
+ *
+ * @param message.msg {number} The message to be sent.


No doc for the actual message parameter.

In fact, why not unroll these into individual parameters, since they're all required?

stegru · 2018-08-15T14:28:32Z

gpii/node_modules/registrySettingsHandler/src/RegistrySettingsHandler.js

            newValue: newValue.value
        };
    });
+


Stray newline

stegru · 2018-08-15T14:34:25Z

gpii/node_modules/windowMessages/src/windowMessages.js

+ *
+ * @param message.msg {number} The message to be sent.
+ * @param message.wparam {Object} The wparam to be passed to SendWindowNotifyMessageW function.
+ * @param message.lparam {Object} The lparam to be passed to SendWindowNotifyMessageW function.


wparam should be {Number}, and lparam a {String} (considering line 255)

JavierJF added 4 commits December 6, 2017 16:50

Merge branch 'master' of https://github.com/GPII/windows

34b733c

GPII-2729: New imported function and constants necessary for sending …

ddf4d46

…windows messages

GPII-2729: Added the possibility of sending windows messages in the R…

3587a73

…egistrySettingsHandler

GPII-2729: Changed the package.json reference to point to the correct…

35c6447

… universal

javihernandez added the Needs Work label Dec 8, 2017

kaspermarkus requested a review from amb26 December 13, 2017 17:54

stegru reviewed Dec 19, 2017

View reviewed changes

Merge branch 'master' of https://github.com/GPII/windows into GPII-2729

2d015a5

Merge branch 'master' of https://github.com/GPII/windows into GPII-2729

eaf6a57

GPII-2729: Several changes

cb2854e

+ Removed the ability to specify windows handle in message feature. + Fixed some linter issues.

Merge branch 'master' of https://github.com/GPII/windows into GPII-2729

2bef466

JavierJF added 3 commits June 14, 2018 15:37

GPII-2729: Moved function to send messages to windows to WindowsMessa…

e138b08

…jes module

GPII-2729: Added new line at eof

4347c0f

GPII-2729: Removed the functionality of sending windows messages from…

6aa77c3

… the RegistrySettingsHandler

GPII-2729: Added unit test for sendWindowMessage function

966d92a

GPII-2729: Fixed linter errors

52fd938

stegru reviewed Jun 19, 2018

View reviewed changes

JavierJF added 3 commits June 20, 2018 13:02

GPII-2729: Added comment to the new sendWindowMessage function

aebd5fb

GPII-2729: Changed message check in favour of in place switch-case style

7c3f1d0

GPII-2729: Fixed test implementation with new one proposed by @stegru

f195790

GPII-2729: Added line at eof fixing linter issues

4d58f5b

Merge branch 'master' of https://github.com/GPII/windows into GPII-2729

f8bbd23

stegru reviewed Aug 15, 2018

View reviewed changes

JavierJF closed this Oct 13, 2020

GPII-2729: Onboard Windows 10 Internationalization settings #158

GPII-2729: Onboard Windows 10 Internationalization settings #158

Uh oh!

Conversation

JavierJF commented Dec 8, 2017

Uh oh!

gpii-bot commented Dec 8, 2017

Uh oh!

kaspermarkus commented Dec 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stegru commented Dec 20, 2017

Uh oh!

gpii-bot commented Feb 17, 2018

Uh oh!

stegru commented Apr 13, 2018

Uh oh!

gpii-bot commented Apr 13, 2018

Uh oh!

stegru commented Apr 13, 2018

Uh oh!

gpii-bot commented Jun 6, 2018

Uh oh!

gpii-bot commented Jun 8, 2018

Uh oh!

gpii-bot commented Jun 11, 2018

Uh oh!

gpii-bot commented Jun 14, 2018

Uh oh!

gpii-bot commented Jun 14, 2018

Uh oh!

gpii-bot commented Jun 14, 2018

Uh oh!

gpii-bot commented Jun 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gpii-bot commented Jun 20, 2018

Uh oh!

stegru commented Jun 20, 2018

Uh oh!

gpii-bot commented Jun 20, 2018

Uh oh!

gpii-bot commented Jun 20, 2018

Uh oh!

stegru commented Jun 20, 2018

Uh oh!

gpii-bot commented Jun 20, 2018

Uh oh!

gpii-bot commented Jul 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants