773159 - Add support for returning the title and type of the active chrome window

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

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

+++ This bug was initially created as a clone of Bug #754226 +++

It would be great if beside the page title Marionette can also return the actual title and type of the chrome window, when it's in chrome mode. In case of Mozmill we would need it a lot.

title: window.document.documentElement.getAttribute('title')
type:  window.document.documentElement.getAttribute('windowtype')

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

For setting the title:

https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-actors.js#1076

If we're in chrome context, then we'll need to get the current window (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-actors.js#407) and return the document.documentElement.getAttribute('title') value to the client.

To test this, you'll have to write a test.xul file and add it to (mozilla-central)/testing/marionette/client/marionette/www/, and set the chrome window's title to something. To test it, write a test in (mozilla-central)/testing/marionette/client/marionette/tests/unit/ and in your test, you should switch to chrome context, load this page, and verify that the title is what you expect. You'll need to add this test to the test manifest (mozilla-central)/testing/marionette/client/marionette/tests/unit/unit-test.ini

For the window type:

Henrik, I don't think we have a method for getting the window type in webdriver. We can add it as a special function, since it requires knowledge of the current chromewindow, and it's not fun to have that in an execute_script call. dburns, thoughts?

Comment 2

[Malini Das [:mdas] - Away, not checking bugmail]

an example of a .xul file: http://mxr.mozilla.org/mozilla-central/source/dom/ipc/test.xul

Comment 3

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

(In reply to Malini Das [:mdas] from comment #1) 
> Henrik, I don't think we have a method for getting the window type in
> webdriver. We can add it as a special function, since it requires knowledge
> of the current chromewindow, and it's not fun to have that in an
> execute_script call. dburns, thoughts?

Not sure I understand that. If we can retrieve the window title, what's so complicated to do the same for the window type? Similar to the title it is a DOM attribute on the window itself (see my comment 0). Can't we add a get(Window)Type() to the actor?

Comment 4

[mathieu bultel:mbu]

hello,

i'll try to focus on this issue this week end.

Thx Malini for the details.

Comment 5

[David Burns :automatedtester]

(In reply to Malini Das [:mdas] from comment #1)
> For setting the title:
> 
> https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-
> actors.js#1076
> 
> If we're in chrome context, then we'll need to get the current window
> (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/
> marionette-actors.js#407) and return the
> document.documentElement.getAttribute('title') value to the client.
> 
> To test this, you'll have to write a test.xul file and add it to
> (mozilla-central)/testing/marionette/client/marionette/www/, and set the
> chrome window's title to something. To test it, write a test in
> (mozilla-central)/testing/marionette/client/marionette/tests/unit/ and in
> your test, you should switch to chrome context, load this page, and verify
> that the title is what you expect. You'll need to add this test to the test
> manifest
> (mozilla-central)/testing/marionette/client/marionette/tests/unit/unit-test.
> ini
> 
> For the window type:
> 
> Henrik, I don't think we have a method for getting the window type in
> webdriver. We can add it as a special function, since it requires knowledge
> of the current chromewindow, and it's not fun to have that in an
> execute_script call. dburns, thoughts?

That's fine by me to add a new getWindowType() to the actor. We don't need it on a listener though since this is more for Chrome automation unless there is a good use case that I cant think of

Comment 6

[mathieu bultel:mbu]

I add new function getWindowType() to the actor and i also add a function get_window_type() to marionette.py which call actor function.

I'll put the patch tomorrow evening prehaps... I just want to add unit tests for getWindowType to the marionette client.

For the xul file, i update one on /testing/marionette/client/marionette/chrome/test.xul 
Is there any pb with that, or not ?

Comment 7

[mathieu bultel:mbu]

(and i also fix the getTitle issue of course ;) )

Comment 8

[mathieu bultel:mbu]

Attachment: First submission for getting support of getTitle and getWindowType in actor part of marionette

First submission for getting support of getTitle in chrome context and getWindowType.

Comment 9

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 752618 [details] [diff] [review]
First submission for getting support of getTitle and getWindowType in actor part of marionette

Review of attachment 752618 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome work, mbu! This is looking really good and I'm glad you were able to figure out how things work so quickly!

I added a few suggestions, but it also seems that this patch can't be applied to the latest mozilla-central code cleanly. I keep getting errors when I try to apply it. Can you fix the errors in your patch? To do this, do:

hg qpop # to pop the patch
hg pull -u # to update mozilla-central to the latest commit
hg qpush # to push the patch back

and then you can resolve any conflicts, then update patch with 'hg qrefresh'.

::: testing/marionette/client/marionette/tests/unit/test_window_title.py
@@ +20,5 @@
> +        MarionetteTestCase.tearDown(self)
> +
> +    def test_get_chrome_title(self):
> +        title = self.marionette.execute_script("return window.document.documentElement.getAttribute('title');")
> +        self.assertEqual(title, self.marionette.title)

can you add a check that the title is equal to "Title Test" as well?

::: testing/marionette/client/marionette/www/testTitle.html
@@ +10,5 @@
> +</head>
> +<body>
> +  <p> Check the Title
> +</body>
> +</html>

instead of adding a new test file, you can use test.html, and use its title.

Comment 10

[Malini Das [:mdas] - Away, not checking bugmail]

I'll wait for that patch before finishing my review, since I'll need it to test that everything is working when run against the phone.

Comment 11

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 752618 [details] [diff] [review]
First submission for getting support of getTitle and getWindowType in actor part of marionette

Review of attachment 752618 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome work, mbu! This is looking really good and I'm glad you were able to figure out how things work so quickly!

I added a few suggestions, but it also seems that this patch can't be applied to the latest mozilla-central code cleanly. I keep getting errors when I try to apply it. Can you fix the errors in your patch? To do this, do:

hg qpop # to pop the patch
hg pull -u # to update mozilla-central to the latest commit
hg qpush # to push the patch back

and then you can resolve any conflicts, then update patch with 'hg qrefresh'.

::: testing/marionette/client/marionette/tests/unit/test_window_title.py
@@ +20,5 @@
> +        MarionetteTestCase.tearDown(self)
> +
> +    def test_get_chrome_title(self):
> +        title = self.marionette.execute_script("return window.document.documentElement.getAttribute('title');")
> +        self.assertEqual(title, self.marionette.title)

can you add a check that the title is equal to "Title Test" as well?

::: testing/marionette/client/marionette/www/testTitle.html
@@ +10,5 @@
> +</head>
> +<body>
> +  <p> Check the Title
> +</body>
> +</html>

instead of adding a new test file, you can use test.html, and use its title.

Comment 12

[Malini Das [:mdas] - Away, not checking bugmail]

Heh whoops, I republished my draft response

Comment 13

[mathieu bultel:mbu]

Attachment: new patch according to Malini review, hope it will be good

Comment 14

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 753424 [details] [diff] [review]
new patch according to Malini review, hope it will be good

Review of attachment 753424 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great and runs well!

I just realized that since we're testing on .xul pages, there is one thing we'll have to add. We run these tests on b2g phones/emulators, and we can't run xul on them successfully, so we'll have to skip those tests. We'll want to skip test_window_type.py completely, but for test_window_title.py, we can still run the content test with no problem. Instead of skipping the whole test, we can flag part of the test with the @skip_if_b2g decorator. I've added comments on how to do this.

Thanks!

::: testing/marionette/client/marionette/tests/unit/test_window_title.py
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +from marionette_test import MarionetteTestCase

Can you change this to:

from marionette_test import MarionetteTestCase, skip_if_b2g

@@ +3,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +from marionette_test import MarionetteTestCase
> +
> +class TestTitleChrome(MarionetteTestCase):

then above this class, can you add this:

@skip_if_b2g

::: testing/marionette/client/marionette/tests/unit/unit-tests.ini
@@ +86,5 @@
>  [test_screenshot.py]
>  [test_cookies.py]
>  b2g = false
> +[test_window_title.py]
> +[test_window_type.py]

can you flag this as

b2g = false

Comment 15

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Malini Das [:mdas] from comment #14)
> @@ +86,5 @@
> >  [test_screenshot.py]
> >  [test_cookies.py]
> >  b2g = false
> > +[test_window_title.py]
> > +[test_window_type.py]
> 
> can you flag this as
> 
> b2g = false

I meant, just flag [test_window_type.py] with b2g = false

Comment 16

[mathieu bultel:mbu]

Attachment: Add b2g option into manifest

Comment 17

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 753943 [details] [diff] [review]
Add b2g option into manifest

Review of attachment 753943 [details] [diff] [review]:
-----------------------------------------------------------------

great, thanks! I'll push to m-i soon

Comment 18

[Malini Das [:mdas] - Away, not checking bugmail]

m-i's busted, will land when it's open

Comment 19

[Malini Das [:mdas] - Away, not checking bugmail]

pushed to m-i:
https://hg.mozilla.org/integration/mozilla-inbound/rev/291792974254

Comment 20

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/291792974254

Comment 21

[Malini Das [:mdas] - Away, not checking bugmail]

We need to land this in v1.0.1 and mozilla-b2g18 for test harness updates, with a=test-only

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9da66738d78d
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d7cb2e4b99ab