Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add control flow to the IDE #140

Closed
wants to merge 3 commits into from
Closed

WIP: Add control flow to the IDE #140

wants to merge 3 commits into from

Conversation

corevo
Copy link
Member

@corevo corevo commented Apr 4, 2018

Designing flow control for the IDE, including playback, and export.

  • break
  • continue
  • do endDo
  • if else end
  • times end
  • while end

@Jongkeun
Copy link
Contributor

Do you have any schedule at this pr?
I'm looking forward this commands and storeEval.
Let me know the schedule if you have?

@Jongkeun
Copy link
Contributor

I've already used flow control on my old customized selenium.
I used sideflow.
Is there something I can help you?

@Jongkeun
Copy link
Contributor

Jongkeun commented May 9, 2018

I already made flow control, but not yet done. And I even made gotoif and label.
Absolutely your result is better than me.
The reason that I made separately is I needed it early.

Additionally we need 'gotoif' and 'label'.
Because we should make it run old scripts on the new selenium-ide.
Although remove gotoif, that is not matter if gotoif is migrated to if automatically.

But I agree that eval has a problem too.
Nevertheless we need the command.
I knew eval is not recommend but I am using it by changing extension policy.
Also Katalon is using these command.
Because they have known it is important to user.
I want you to consider it and if you don't want it, think about alternative.

As one of user, I want new selenium-ide to be the best program of several selenium-ide programs.
I would help you as possible as I can.

@corevo
Copy link
Member Author

corevo commented May 9, 2018

I didn't know you we're working on this, looks amazing.
About goto and label we have this, it will convert goto and label to while and if.
About eval, don't get me wrong, I want Selenium IDE to be the best thing out there, but as of this moment, Mozilla sent us a message demanding to remove it.
After we remove the remnants of the old eval system, we'll talk with them about introducing a safer one.
I am unsure to what agreement Katalon has arrived with Mozilla about unsafe code injections, but it is not something Mozilla have discussed with us.

@corevo
Copy link
Member Author

corevo commented May 11, 2018

@Jongkeun I was thinking for a bit, why did you change the security policy?
Do you eval the conditions inside the background script?
You know that would enable access to chrome.debugger and in extension to any origin request?
There's a huge potential security risk in doing this.

@Jongkeun
Copy link
Contributor

Jongkeun commented May 11, 2018

@corevo So I restored policy.
And I changed the way from calling eval directly to calling to use sendMessage to commands-api.
I put doEval at selenium-api.js.
There are many reasons for the change.
But the important reason is I have to use storedVars.
So if I wanna use eval I am calling sendMessage with eval's condition by using the ext-command.
And I get the returnValue about the condition.

Anyway, my flow control commands are at extension side.
Because I should handle TestCase.
I couldn't handle TestCase on browser side.

What are you worried about?
Do you think the user will become an attacker? Or will be a victim?

@Jongkeun
Copy link
Contributor

When I called eval on browse side, there was no error or warning.

@corevo
Copy link
Member Author

corevo commented May 13, 2018

I think the user may well become a victim.
I think it's highly plausible to see someone as a joke tell a user, put this line of code here, and cause a mess, or worse.
More than that, changing the csp will show unfriendly alerts to the users, which I'd like to avoid.

@Jongkeun
Copy link
Contributor

what kind of alerts or warning?
I didn’t have any alert even if I didn’t change csp.

@corevo
Copy link
Member Author

corevo commented May 13, 2018 via email

@Jongkeun
Copy link
Contributor

If the debug and the publish are different, that is problem as you said.
I didn’t know about that.

@Plotist Plotist mentioned this pull request May 15, 2018
@corevo corevo closed this Aug 20, 2018
@corevo corevo deleted the control-flow branch March 20, 2019 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants