1127831 - Implement handling of tabs

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This bug will cover the light-weight handling of tabs compared to bug 1121705. It will not make use of any event and observer code, which the other bug is blocked on.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

A current WIP of the functionality is available at:
https://github.com/mozilla/firefox-ui-tests/pull/56

I got all the basics working so far. Means opening and closing of tabs work. The same for switching between tabs. But there is still a bit of work to do:

* Fully implement all possible trigger methods for opening and closing tabs
* Add assertions to methods which ensure that the trigger action was successful
* Check how we have to handle the focus and automatically switch to newly opened tabs
* Implement the remaining methods which exist as skeletons in the tab module
* Get more tests written, also with two open browser windows

Chris, if you want to continue please do so and let me know about your branch. That way I can re-import your changes and continue on Monday. Please keep in mind that there is a follow-up bug, which is about getting all the remaining tabbar features in. Those things are trivial and I can surely work on those on Monday. So we should be set for Tuesday.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Not sure why but all the toolbar tests fail on travis for this above branch. You might wanna have a look at this first. Maybe I missed something to correctly rebase.

https://travis-ci.org/mozilla/firefox-ui-tests/builds/48987686

Comment 3

[Chris Manchester (limited bugmail, email directly)]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Not sure why but all the toolbar tests fail on travis for this above branch.
> You might wanna have a look at this first. Maybe I missed something to
> correctly rebase.
> 
> https://travis-ci.org/mozilla/firefox-ui-tests/builds/48987686

This is the same problem I had to work around trying to merge the locationbar library. We end up in "content" context by tearDown when a test is skipped. This seems like a bug in marionette automation, but easy enough to work around.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

I got further with this in the top three commits at: https://github.com/chmanchester/firefox-ui-tests/tree/tabs

I think this is looking good. From the list in comment 1, there might be more trigger methods to implement, and more tests would be in order. I'm not sure about automatically switching into tabs, but I suspect because the effect of this is primarily for which tab receives content commands, and we'll primarily work in chrome, the more relevant thing for us will be tab selection in the UI.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Chris, thanks for the additionally work. I will have a look at it soon and continue with the remaining bits now. I will have a review request for you later today.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: github_pull_request.txt

Chris, please have a look at the Github pull request. I just uploaded the final version of my patch. Thanks.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8558106 [details] [review]
github_pull_request.txt

I left a few comments in Github that should be addressed if possible, no hard blockers.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Chris for the review. I made all the required changes and merged the PR as:
https://github.com/mozilla/firefox-ui-tests/commit/37736b3257e761e6c05fa3033edd3ee4670bbd14

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I had to push a follow-up for this because the Tab documentation was missing on readthedocs:
https://github.com/mozilla/firefox-ui-tests/commit/09fe9a07f5b9f6fc870a199d7a58114a9a0a3675