Closed Bug 1414221 Opened 4 years ago Closed 4 years ago

Update README for moz:webdriverClick capability

Categories

(Testing :: geckodriver, enhancement)

58 Branch
enhancement
Not set
normal

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file)

We should add some information about this capability, so that people know what to do in case issues come up.
Landing the upcoming patch would depend on our agreement if it should ride the train, or to get it uplifted to Firefox 57 via bug 1321516.
Depends on: 1321516
Comment on attachment 8924932 [details]
Bug 1414221 - Update README.de for moz:webdriverClick capability.

https://reviewboard.mozilla.org/r/196196/#review201372

I would like to get some feedback for the wording, and not land it right away on my own. Thanks.
Comment on attachment 8924932 [details]
Bug 1414221 - Update README.de for moz:webdriverClick capability.

https://reviewboard.mozilla.org/r/196196/#review201380
Attachment #8924932 - Flags: review?(dburns) → review+
Thanks David. What I just noticed is that I missed to add the table with info about this capability similar to moz:firefoxOptions. I think it's useful even it's a single row only. I will update the patch.
Well, this capability is a boolean, and as such it doesn't have any other atomic nor object included. So a table doesn't make sense. I will push as it is.

Andreas, I assume we want to update the README.md file on Github as soon as we make changes to the file on m-c, and don't want to wait for the next release?
Flags: needinfo?(ato)
Comment on attachment 8924932 [details]
Bug 1414221 - Update README.de for moz:webdriverClick capability.

https://reviewboard.mozilla.org/r/196196/#review201456

Thanks for documenting this.  I think it looks mostly fine, but the language could be tightened up.

::: testing/geckodriver/README.md:202
(Diff revision 1)
>  
>  
>  Firefox capabilities
>  ====================
>  
> -geckodriver also supports a capability named `moz:firefoxOptions`
> +Geckodriver also supports capabilities with the `moz:` prefix, which can

s/Geckodriver/geckodriver/

::: testing/geckodriver/README.md:288
(Diff revision 1)
> +moz:webdriverClick
> +------------------
> +
> +A boolean value to indicate which kind of interactability checks to run
> +when performing a click on elements. For Firefoxen prior to version 58.0 some
> +legacy code as imported from an older version of [Firefox Driver] was in use.

s/Firefox Driver/FirefoxDriver/g

::: testing/geckodriver/README.md:290
(Diff revision 1)
> +With Firefox 58 the interactability checks as required by the [WebDriver]
> +specification are enabled by default now. It means that there are checks

s/now//

::: testing/geckodriver/README.md:291
(Diff revision 1)
> +specification are enabled by default now. It means that there are checks
> +for obscured elements in-place now, and elements located outside the viewport

Replace:

> It means that there are checks for obscured elements in-place now,

With:

> This means geckodriver will additionally check if an element is obscured by another when clicking,

::: testing/geckodriver/README.md:292
(Diff revision 1)
> +for obscured elements in-place now, and elements located outside the viewport
> +are automatically scrolled into view.

We’ve always scrolled elements outside the viewport into view.

::: testing/geckodriver/README.md:295
(Diff revision 1)
> +Those changes could cause some extra errors to be thrown, or the click to
> +behave differently. In most cases the test in question might have to be updated

Perhaps this?

> Because of this change in behaviour, we are aware that this change could cause some extra errors to be returned

::: testing/geckodriver/README.md:297
(Diff revision 1)
> +so it's conform with the new checks. But if it's a problem with Firefox please
> +raise an issue in the geckodriver [issue tracker].

Not sure I understand that last paragraph.  What are you trying to say?

::: testing/geckodriver/README.md:300
(Diff revision 1)
> +Those changes could cause some extra errors to be thrown, or the click to
> +behave differently. In most cases the test in question might have to be updated
> +so it's conform with the new checks. But if it's a problem with Firefox please
> +raise an issue in the geckodriver [issue tracker].
> +
> +To temporarily disable the webdriver conformant checks use `false` as value

s/webdriver/WebDriver/
Attachment #8924932 - Flags: review+
(In reply to Henrik Skupin (:whimboo) from comment #6)

> I assume we want to update the README.md file on Github as soon as
> we make changes to the file on m-c, and don't want to wait for the
> next release?

Sure.
Flags: needinfo?(ato)
Comment on attachment 8924932 [details]
Bug 1414221 - Update README.de for moz:webdriverClick capability.

https://reviewboard.mozilla.org/r/196196/#review201456

> We’ve always scrolled elements outside the viewport into view.

Wow, so `isInView` != `isVisible`. I just missed that by comparing the code.

> Not sure I understand that last paragraph.  What are you trying to say?

I reworded to make it better. It was indeed a bit confusing.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fa3745bff6ac
Update README.de for moz:webdriverClick capability. r=ato,automatedtester
https://hg.mozilla.org/mozilla-central/rev/fa3745bff6ac
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.