Allow Fill,Type Text to resolve variables too

[stdedos]

Likewise to

    Fill Secret                 input#pass      $password

allow also

    Type Text                   input#user      $username

to be resolvable.

I thought it should've been RF's job to provide you the "conversion", but it seems that's not the case.

(I could try to code it myself; I am more-or-less looking for approval that this is indeed accepted) (See: https://robotframework.slack.com/archives/C015KB1QSDN/p1621453094010200)

[mkorpela]

Why can’t you do ${username} ?

[stdedos]

It's not that "I cannot do it"; I think the interfaces should be the same.

i.e. I don't understand / it doesn't make sense to me to force one "Fill" function to use ${x} and another to use $x.

[mkorpela]

I believe secret keywords should accept ${password} .. I think the shorthand without brackets there was just another way to define those.

[stdedos]

It does accept it, but it cries out deprecation. In my books, this is as good as removed (or "the next thing to remove").

From https://github.com/MarketSquare/robotframework-browser/issues/893#issuecomment-812051531:

*** Settings ***
Library    Browser

*** Test Cases ***

Fill Secret With Variable
  ${secret}    Set Variable    me@gmail.com

  New Browser    chromium     headless=false
  New Context    locale=nl-NL
  New Page    https://accounts.google.com/signin
  Fill Secret     id=identifierId     ${secret}

robot robot.robot
==============================================================================
Robot
==============================================================================
[ WARN ] Direct assignment of values as 'secret' is deprecated.Use variables or environment variables instead.
Fill Secret With Variable                                             | PASS |
------------------------------------------------------------------------------
Robot                                                                 | PASS |

I am not going to accept that flying in any project; personal or otherwise.

[mkorpela]

Ok. So the bug is that the variable use generates the warning.

[mkorpela]

That is still a bit different issue. Warnings are generated because of the common failure pattern of writing secrets to logs.

[stdedos]

Yes. And since this is a very loud warning, IMHO it will lead people writing

    Fill Secret                 input#pass      $password

which will then (again IMHO) make them feel puzzled when

    Type Text                   input#user      $username

won't work.

I understand the rationale behind "the issue that lead to #893". I think that similar methods should be able to do things similarly (to some extent).

[mkorpela]

This is the issue where warnings starts https://github.com/MarketSquare/robotframework-browser/issues/421

[mkorpela]

As @aaltat wrote "Also for us, in the library level, it's not possible to influence RF running context in that level.". @stdedos: If you want to help with this issue, I would first suggest at looking in making secret keyword not giving the warning when using variables. It is in my opinion a non-optimal solution to force users to use $variable so that library can do the parsing instead of Robot Framework. It would be better to use standard variables ${variable}.

[stdedos]

As @aaltat wrote "Also for us, in the library level, it's not possible to influence RF running context in that level.".

I think @aaltat is correct, and therefore $secret suggestion cannot be reverted. I also think that the warning is well warranted; if the library tries to protect me against secret spillage, I want any of it. Library cannot enforce RF workings; but it can dictate itself. (I don't know why Input Password https://robotframework.org/SeleniumLibrary/SeleniumLibrary.html#Input%20Password is not enough - I "may be" okay secrets spilling in TRACE logging). Therefore, the solution forward I think is making Fill/Type variables accept $variables.

[aaltat]

SeleniumLibrary Input Password will leak the secret in many ways. In SeleniumLibrary if one wants to debug issues, one needs to raise logging level so that Selenium logging is visible and that will leak the secrets. SeleniumLibrary is not good role model here. In Browser context, our secret hiding is not bullet proof either, we will leak secrets if Playwright debug logs is enabled.

I am not favor of enabling $ or % prefix resolving in all Fill/Type because the lack of third party support. The $secret or %secret is seen as string in RF data syntax. So if you make a typo, IDE and other tool will not complain, because tools will think it is string. Only in run time, keyword will complain, which is annoying.

There is work to make the feature and maintain the syntax in our side. Also it opens a door for this implementation to leak in other keywords, where it is not needed in my mind. I understand your need for symmetry, but in this case, I think that symmetry is not the best solution.

Therefore I am not in favor to have this in the Type/Fill keywords.

[Snooz82]

Just to make it more clear for me an others?!

This is NOT a matter of Fill vs Type! It is about Fill Text & Type Text vs. Fill Secret & Type Secret

And if we would implement both keyword groups to do the same, there would not be a need for these two different kinds.

I think as Mikko said, there is one error. That we write in the warning that "variables" are allowed. That is not really the case. The ... Secret keywords are only silent, when you use the "python variable syntax"

I would recommend to close that issue. Sorry

[Snooz82]

@mkorpela I think the warning is correct, with current "bug" in Robot Framework that the unresolved variables do not work correctly.

So the warning text should be changed.

As you can see in the following screenshot, trace spoils variables.

But it does not spoil the "python variable syntax"

[aaltat]

@Snooz82 I do not see how the warning message should be changed?

[Snooz82]

@aaltat because it says "use variables or env variables instead" But that does not help! You have to use special variable syntax to work. $var instead of ${var}

That should be in the warning.

[aaltat]

Closing due #1070