_WD_SetElementValue $_WD_OPTION_Advanced usage

mlipok commented 2 years ago

Pull request

Proposed changes

Better $_WD_OPTION_Advanced implementation in _WD_SetElementValue

Checklist

Put an x in the boxes that apply. If you're unsure about any of them, don't hesitate to ask. We are here to help!
This is simply a reminder of what we are going to look for before merging your code.

[x] I have read and noticed the CODE OF CONDUCT document
[x] I have read and noticed the CONTRIBUTING document
[ ] I have added necessary documentation or screenshots (if appropriate)

Types of changes

Please check x the type of change your PR introduces:

[x] Bugfix (change which fixes an issue)
[ ] Feature (change which adds functionality)
[ ] Code style update (formatting, renaming)
[x] Refactoring (functional, structural)
[ ] Documentation content changes
[ ] Other (please describe)

What is the current behavior?

_WD_SetElementValue with $_WD_OPTION_Advanced still not always properly sets value.

What is the new behavior?

$_WD_OPTION_Advanced fires change event for this reason all background processing are processed.

Additional context

None

System under test

not related. But mostly issue with setting value occurs on Firefox in case when Firefox was visible but was obscured by another window.

Danp2 commented 2 years ago

_WD_SetElementValue with $_WD_OPTION_Advanced still not always properly sets value.

When making such a statement, it would be helpful if you included code that demonstrates the issue.

Also, your modification is a complete rewrite of the Javascript involved, and it may not handle some of the situations that work correctly with the current implementation.

mlipok commented 2 years ago

When making such a statement, it would be helpful if you included code that demonstrates the issue.

I will try to find some public available site which will have the same issue.

Also, your modification is a complete rewrite of the Javascript involved, and it may not handle some of the situations that work correctly with the current implementation.

Do you propose to add new separate option ? Something like $_WD_OPTION_FireEvent ?

mlipok commented 2 years ago

btw. I hope maybe @TheDcoder will take a look and give some opinion about this proposals.

Danp2 commented 2 years ago

Do you propose to add new separate option ? Something like $_WD_OPTION_FireEvent ?

No. I was simply pointing out that your proposed solution may correct one problem, but others that didn't previously exist.

I hope maybe @TheDcoder will take a look and give some opinion about this proposals.

That would be good, especially if you can show us where the current implementation is failing.

mlipok commented 2 years ago

Using yours example from here: https://www.autoitscript.com/forum/topic/205786-multiline-string-frome-a-txt-file/?do=findComment&comment=1481423

As for now I tested it with: https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_textarea

var myelement = document.querySelector('#w3review');
var myvalue = '12345';
Object.getOwnPropertyDescriptor(myelement.__proto__, 'value').set.call(myelement, myvalue);
myelement.dispatchEvent(new Event('input', { bubbles: true }));

and

var myelement = document.querySelector('#w3review');
var myvalue = '123456';
myelement.value = myvalue;
var event;
if (typeof(Event) === 'function') {
 event = new Event('change');
} else {
 event = document.createEvent('Event');
 event.initEvent('change', true, true);
};
myelement.dispatchEvent(event);

both snippets works well. Searching for public website which will not work with bubbles but will work with change

TheDcoder commented 2 years ago

Both input and change events are fired by the browser naturally, so we should dispatch both of them, it should also not break older code. bubbles is also required in most cases and actually mimic's the browsers behavior for natural events, so it should always be true :smile:

By the way, I think initEvent is deprecated. The usual way to create an event with custom options is to use the appropriate constructor. Should I make a small snippet with all of my suggestions?

mlipok commented 2 years ago

Should I make a small snippet with all of my suggestions?

Yes. Please provied snippet. Then I will be able to test it on my side in the restricted website.

mlipok commented 2 years ago

By the way, I think initEvent is deprecated.

Some of the code was related to InterentExplorer, so it may be handy when you use MSEdge in InternetExplorer mode.

TheDcoder commented 2 years ago

This is what I had in my mind:

Object.getOwnPropertyDescriptor(arguments[0].__proto__, 'value').set.call(arguments[0], arguments[1]);
arguments[0].dispatchEvent(new Event('input', {bubbles: true}));
arguments[0].dispatchEvent(new Event('change', {bubbles: true}));

Some of the code was related to InterentExplorer, so it may be handy when you use MSEdge in InternetExplorer mode.

I see, I'm not familiar with MSEdge or its InternetExplorer mode, but in my opinion this is not in the scope of this project. A separate helper UDF can be created to support IE mode, we should not add this code to the main UDF.

mlipok commented 2 years ago

I will be able to login to the restricted website to test this $_WD_OPTION_Advanced TheDcoder fix in next few days.

Currently it works fine in normal cases.

mlipok commented 2 years ago

I will be able to login to the restricted website to test this $_WD_OPTION_Advanced TheDcoder fix in next few days.

Already tested. Works but not so good as I was expecting.

My issue may be related to FF version, driver version, windows version, any other ENV setting.

Here is what I'm having a problem (more or less accurate description).

When FF window is visible (aka TOPMOST) then all is fine. When FF window is covered by another window, then value is set (visually you can notice that it was set) but when program click "SUBMIT" button then it turns out that the value has not been set correctly.

TheDcoder commented 2 years ago

Works but not so good as I was expecting.

Does your code work when the window is not active? My code should also do what your code does, so not sure what is going on.

mlipok commented 2 years ago

I propose that we merge this PR with the addition from @TheDcoder.

Agree

@mlipok You may need to file a separate bug report with Mozilla if you want FF to work correctly.

Firstly I need to do more test on my side.

mlipok commented 2 years ago

Both input and change events are fired by the browser naturally, so we should dispatch both of them, it should also not break older code. bubbles is also required in most cases and actually mimic's the browsers behavior for natural events, so it should always be true 😄

as reference:

Danp2 / au3WebDriver