Closed Bug 1243415 Opened 9 years ago Closed 8 years ago

Have chrome screen capture code use the new capture.js module

Categories

(Remote Protocol :: Marionette, defect)

Product:
Remote Protocol
Component:
Marionette
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox52 fixed, firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: ato, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-server)

Attachments

(12 files, 1 obsolete file)

Bug 1243415 - Capture methods should use the window as parameter.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Remove B2G related code from screenshot methods.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Make use of capture.js in chrome scope.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Add highlighting of elements for screenshots in chrome scope.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Add hash format support for screenshots in chrome scope.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Add support to take full screenshots in chrome scope.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Check for a valid DOMWindow for screenshot in chrome scope.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1243415 - Improve screenshot unit tests and add tests for chrome scope.
58 bytes, text/x-review-board-request
ato
: review+
Details
screenshot1.png (expected failed)
5.91 KB, image/png
Details
screenshot2.png (expected failed)
5.58 KB, image/png
Details
screenshot1.png (unexpected pass)
3.00 KB, image/png
Details
screenshot2.png (unexpected pass)
3.00 KB, image/png
Details 
The screen capture code in chrome space is still not using the new testing/marionette/capture.js module.  There is not reason it cannot share the same screenshot code as with content, but likely a few modifications will need to happen.

I suspect this has caused issues such as bug 1140542.
Blocks: 1247600
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #8817411 - Flags: review?(dburns)
Tests for the try build are looking fine, except that we have some strange element sizes on Linux:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f9fc451512f&selectedJob=32380579

A follow-up patch for the tests will fix that by converting them to int.
Comment on attachment 8817403 [details]
Bug 1243415 - Capture methods should use the window as parameter.

https://reviewboard.mozilla.org/r/97676/#review98076
Attachment #8817403 - Flags: review?(dburns) → review+
Comment on attachment 8817405 [details]
Bug 1243415 - Remove B2G related code from screenshot methods.

https://reviewboard.mozilla.org/r/97678/#review98078
Attachment #8817405 - Flags: review?(dburns) → review+
Comment on attachment 8817407 [details]
Bug 1243415 - Add highlighting of elements for screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97682/#review98080
Attachment #8817407 - Flags: review?(dburns) → review+
Comment on attachment 8817408 [details]
Bug 1243415 - Add hash format support for screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97684/#review98082
Attachment #8817408 - Flags: review?(dburns) → review+
Comment on attachment 8817409 [details]
Bug 1243415 - Add support to take full screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97686/#review98084

::: testing/marionette/driver.js:2409
(Diff revision 1)
>          let el = this.curBrowser.seenEls.get(h, container);
>          highlightEls.push(el);
>        }
>  
> -      let canvas = capture.viewport(this.getCurrentWindow(), highlightEls);
> +      // viewport
> +      if (!id && !full) {

Can the chrome code be larger than the viewport?
Attachment #8817409 - Flags: review?(dburns) → review+
Comment on attachment 8817406 [details]
Bug 1243415 - Make use of capture.js in chrome scope.

https://reviewboard.mozilla.org/r/97680/#review98086

::: testing/marionette/capture.js:94
(Diff revision 1)
>   * @return {HTMLCanvasElement}
>   *     The canvas on which the selection from the window's framebuffer
>   *     has been painted on.
>   */
>  capture.canvas = function(win, left, top, width, height, highlights=[]) {
> +  let scale = win.devicePixelRatio;

Why have we added the scaling code? Just curious for my own learning
Attachment #8817406 - Flags: review?(dburns) → review+
Comment on attachment 8817410 [details]
Bug 1243415 - Check for a valid DOMWindow for screenshot in chrome scope.

https://reviewboard.mozilla.org/r/97688/#review98088
Attachment #8817410 - Flags: review?(dburns) → review+
Comment on attachment 8817406 [details]
Bug 1243415 - Make use of capture.js in chrome scope.

https://reviewboard.mozilla.org/r/97680/#review98086

> Why have we added the scaling code? Just curious for my own learning

This is probably needed because of pixel scaling on retina displays.  I’ve seen similar code in devtools.
Comment on attachment 8817409 [details]
Bug 1243415 - Add support to take full screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97686/#review98084

> Can the chrome code be larger than the viewport?

Yes, I had to try myself for a while to figure that out. So when you load a HTML test page into a chrome window - something a new test will do - the scrollbars will appear, if the window is opened too small. With pure XUL it might have also been possible somehow but I gave up for now.
Comment on attachment 8817406 [details]
Bug 1243415 - Make use of capture.js in chrome scope.

https://reviewboard.mozilla.org/r/97680/#review98086

> This is probably needed because of pixel scaling on retina displays.  I’ve seen similar code in devtools.

Correct. A good overview about how it works you can find eg. here: http://www.quirksmode.org/blog/archives/2012/06/devicepixelrati.html

Generally it's that we do not want to get oversized images created if multiple device pixels map to a single pixel on screen.
I noticed that my changes to the tests even for content has increased the test failures for Fennec a lot. I will try to figure out why that is happening. Most likely this is due to the devicePixelRatio, because failures look like:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f9fc451512f&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=32380259

> Tuples differ: (300, 462) != (600, 924)

Which would mean there is a ratio of 2 used for Android emulator.

Also while looking through code on DXR I noticed that a lot is using the drawing flags like caret, widget layer and such we we had in before.
Blocks: 1297394
Depends on: 1009762
By adding the drawing flags I was able to catch a crash of Firefox for our tests which is tracked as bug 1009762. Here the try results:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7758e3f58733&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=32456548&bugfiler

For now I will disable using 'ctx.DRAWWINDOW_USE_WIDGET_LAYERS'. We can re-enable with the fix of bug 1009762 or once the patch for it landed.
David, beside the remaining review for the tests, I would need a follow-up review for the following changes:
https://reviewboard.mozilla.org/r/97680/diff/1-3/

Thanks.
Flags: needinfo?(dburns)
Attachment #8817411 - Flags: review?(dburns)
The last try build shows two failures for content tests which already existed before and which are about a single line dimension difference for screenshots. I would suggest that we get that fixed in a follow-up bug.

Otherwise I wonder if we should better make use of the hash in most cases of this test. It has two benefits... we do not have to transfer that much data to the client, and on the other side make the tracing log smaller which is enabled for all builds on Treeherder. Before I do the change, I would like to know what others think. The downside will be that if a failure occurred you cannot simply copy and paste the data and open as data url.
Redirecting last question to Andreas given that David will not be around the next days and it would be good to get this patch set landed before Christmas.
Flags: needinfo?(dburns)
Comment on attachment 8817411 [details]
Bug 1243415 - Improve screenshot unit tests and add tests for chrome scope.

https://reviewboard.mozilla.org/r/97690/#review98922

::: testing/marionette/chrome/test_dialog.xul:9
(Diff revision 6)
> -<dialog id="dialogTest"
> -        buttons="accept, cancel"
> -        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
>  
>  
> -  <vbox id="things">
> +<dialog id="testDialog"

Trusting you that these changes are correct.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:33
(Diff revision 6)
> -class ScreenCaptureTestCase(MarionetteTestCase):
> -    def assert_png(self, string):
> -        """Test that string is a Base64 encoded PNG file."""
> -        image = base64.decodestring(string)
> +class ScreenshotTestCase(MarionetteTestCase):
> +
> +    def setUp(self):
> +        super(ScreenshotTestCase, self).setUp()

The original name was intended.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:38
(Diff revision 6)
> +        self._device_pixel_ratio = None
> +
> +    @property
> +    def device_pixel_ratio(self):
> +        if self._device_pixel_ratio is None:
> +            self._device_pixel_ratio = self.marionette.execute_script("""
> +                return window.devicePixelRatio
> +                """)
> +        return self._device_pixel_ratio

Not opening an issue on this, but since we don’t memoise the document element or the Y offset of the window, it’s inconsistent to memoise this.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:54
(Diff revision 6)
> +        value = self.marionette.execute_script("return window.pageYOffset")
> +        return value

Just return the value.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:62
(Diff revision 6)
> +    @property
> +    def viewport_dimensions(self):
> +        return self.marionette.execute_script("""
> +            return [arguments[0].clientWidth,
> +                    arguments[0].clientHeight];
> +            """, script_args=[self.document_element])

Even though I’ve coded it so you can use a list for `script_args`, it’s more Pythonic to use a tuple.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:70
(Diff revision 6)
> +        """Test that screenshot is a Base64 encoded PNG file."""
> +        image = base64.decodestring(screenshot)
>          self.assertEqual(imghdr.what("", image), "png")
>  
> -    def get_image_dimensions(self, string):
> -        self.assert_png(string)
> +    def assert_formats(self, element=None):
> +        element = element or self.document_element

Use `if element is not None` here to avoid a falsiness check.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:96
(Diff revision 6)
> +        self.assertEqual(screenshot_image, screenshot_default)
> +        self.assertEqual(binary1, binary2)
> +        self.assertEqual(hash1, hash2)
> +
> +    def get_element_dimensions(self, element):
> +        return [element.rect["width"], element.rect["height"]]

Return tuple and reuse `element.rect` so that it does not have to be requested twice.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:102
(Diff revision 6)
> +
> +    def get_image_dimensions(self, screenshot):
> +        self.assert_png(screenshot)
> +        image = base64.decodestring(screenshot)
>          width, height = struct.unpack(">LL", image[16:24])
> -        return int(width), int(height)
> +        return [int(width), int(height)]

Return tuple, as it did before.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:105
(Diff revision 6)
> +        return [int(rect[0] * self.device_pixel_ratio),
> +                int(rect[1] * self.device_pixel_ratio)]

Return tuple.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:109
(Diff revision 6)
> +    def scale(self, rect):
> +        return [int(rect[0] * self.device_pixel_ratio),
> +                int(rect[1] * self.device_pixel_ratio)]
> +
>  
> +class TestScreenshotChrome(WindowManagerMixin, ScreenshotTestCase):

Keep old name.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:113
(Diff revision 6)
> -        current_window = self.marionette.current_chrome_window_handle
> +        return self.marionette.execute_script("""
> -        self.marionette.switch_to_window(self.start_window)
> -        with self.marionette.using_context("chrome"):

Return this as a tuple.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:119
(Diff revision 6)
>      def setUp(self):
> -        super(TestScreenCaptureChrome, self).setUp()
> +        super(TestScreenshotChrome, self).setUp()
>          self.marionette.set_context("chrome")
>  
>      def tearDown(self):
>          self.close_all_windows()
> -        super(TestScreenCaptureChrome, self).tearDown()
> +        super(TestScreenshotChrome, self).tearDown()

Keep old name.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:140
(Diff revision 6)
> -    # <window> element, which does not exist for all windows.
> -    def test_secondary_windows(self):
> -        def open_window_with_js():
>              self.marionette.execute_script("""
> -                window.open('chrome://marionette/content/test_dialog.xul', 'foo',
> -                            'dialog,height=200,width=300');
> +                window.open(arguments[0], "", arguments[1]);
> +                """, script_args=[url, features])

Tuple again.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:175
(Diff revision 6)
> +        screenshot_focus = self.marionette.screenshot()
>  
> -class Content(ScreenCaptureTestCase):
> -    @property
> +        self.marionette.execute_script("arguments[0].blur();", script_args=[textbox])
> +        screenshot_no_focus = self.marionette.screenshot()
> -    def body_scroll_dimensions(self):
> -        return tuple(self.marionette.execute_script(
> -            "return [document.body.scrollWidth, document.body.scrollHeight]"))
>  
> -    @property
> -    def viewport_dimensions(self):
> +        self.marionette.close_chrome_window()
> +        self.marionette.switch_to_window(self.start_window)
> -        return tuple(self.marionette.execute_script("""
> -            let docEl = document.documentElement;
> -            return [docEl.clientWidth, docEl.clientHeight];"""))
>  
> -    @property
> +        self.assertNotEqual(screenshot_focus, screenshot_no_focus)

Good test.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:279
(Diff revision 6)
> +class TestScreenshotContent(WindowManagerMixin, ScreenshotTestCase):
>  
>      def setUp(self):
> -        ScreenCaptureTestCase.setUp(self)
> +        super(TestScreenshotContent, self).setUp()
>          self.marionette.set_context("content")
>  
> -    def test_html_document_element(self):
> +    def tearDown(self):
> +        self.close_all_tabs()
> +        super(TestScreenshotContent, self).tearDown()

Keep old name.

::: testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py:291
(Diff revision 6)
> +        return self.marionette.execute_script("""
> +            return [document.body.scrollWidth, document.body.scrollHeight]
> +            """)

Return as a tuple.
Attachment #8817411 - Flags: review?(ato) → review+
Comment on attachment 8817411 [details]
Bug 1243415 - Improve screenshot unit tests and add tests for chrome scope.

https://reviewboard.mozilla.org/r/97690/#review98928

This is great work, but I miss more verbose commit messages.  I think it’s worth pointing out in particular that we now support device pixel ratios for screenshots.  If you could spare some time to expand each commit message, I would be very grateful.
Comment on attachment 8817407 [details]
Bug 1243415 - Add highlighting of elements for screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97682/#review98930

::: testing/marionette/driver.js:2397
(Diff revision 4)
> -      resp.body.value = capture.toBase64(canvas);
> -      break;
> +      let highlightEls = [];
> +
> +      for (let h of highlights) {
> +        let el = this.curBrowser.seenEls.get(h, container);
> +        highlightEls.push(el);
> +      }

You could replace all of this with:

```js
let highlightEls = highlights.map(h => this.curBrowser.seenEls(h, container));
```

Not making this an issue because I tend to like seeing the performance penalties of code, which you get by writing out loops.
Attachment #8817407 - Flags: review+
Blocks: 1075006
Blocks: 1323537
(In reply to Henrik Skupin (:whimboo) from comment #37)
> The last try build shows two failures for content tests which already
> existed before and which are about a single line dimension difference for
> screenshots. I would suggest that we get that fixed in a follow-up bug.

Noted.  I think these are about using the bounding client rects instead of the client rects for calculating the area to capture.

> Otherwise I wonder if we should better make use of the hash in most cases of
> this test. It has two benefits... we do not have to transfer that much data
> to the client, and on the other side make the tracing log smaller which is
> enabled for all builds on Treeherder. Before I do the change, I would like
> to know what others think. The downside will be that if a failure occurred
> you cannot simply copy and paste the data and open as data url.

I think for this test, it is fine to transfer the data.  We want to test that it actually does capture what is intended.  Consumers of Marionette should definitely be encouraged to use hashing, as transferring raw binary data is rather expensive since the Marionette protocol isn’t compressed.

However, I don’t think you should use this bug to follow up on making the tracing logs smaller.  I suspect the gecko.log file can be made substantially smaller though.
Comment on attachment 8817411 [details]
Bug 1243415 - Improve screenshot unit tests and add tests for chrome scope.

https://reviewboard.mozilla.org/r/97690/#review98922

> The original name was intended.

Not sure to what this comment refers to. ScreenCapture? I wouldn't call it that way. Webdriver uses Screenshot and our client implementation too.

> Not opening an issue on this, but since we don’t memoise the document element or the Y offset of the window, it’s inconsistent to memoise this.

devicePixelRatio will never change so I do not see a need to spend extra hundreds of milliseconds to send a command to the server and wait for its return value. It's just lost time.

We don't need it for others because those values can change.

> Just return the value.

Ups, left from debugging.

> Even though I’ve coded it so you can use a list for `script_args`, it’s more Pythonic to use a tuple.

We use lists everywhere. I would suggest to file a new bug to get all this converted at once.

> Use `if element is not None` here to avoid a falsiness check.

I assume you meant `if element is None`. :)

> Keep old name.

See above. This is inconsistent.

> Tuple again.

As mentioned above for consistency right now.
Comment on attachment 8817407 [details]
Bug 1243415 - Add highlighting of elements for screenshots in chrome scope.

https://reviewboard.mozilla.org/r/97682/#review98930

> You could replace all of this with:
> 
> ```js
> let highlightEls = highlights.map(h => this.curBrowser.seenEls(h, container));
> ```
> 
> Not making this an issue because I tend to like seeing the performance penalties of code, which you get by writing out loops.

I assume we have a couple of those instances across the Marionette server code. If you feel that we should change this all can you please file a bug? I wonder how large the performance improvements are in general for such a change.
(In reply to Andreas Tolfsen ‹:ato› from comment #50)
> (In reply to Henrik Skupin (:whimboo) from comment #37)
> > The last try build shows two failures for content tests which already
> > existed before and which are about a single line dimension difference for
> > screenshots. I would suggest that we get that fixed in a follow-up bug.
> 
> Noted.  I think these are about using the bounding client rects instead of
> the client rects for calculating the area to capture.

Whatever this is. In such a case I would expect a difference of 2 pixels but not of 1, given that this should affect the top and bottom.

> I think for this test, it is fine to transfer the data.  We want to test
> that it actually does capture what is intended.  Consumers of Marionette
> should definitely be encouraged to use hashing, as transferring raw binary
> data is rather expensive since the Marionette protocol isn’t compressed.

Ok. Thanks.
Comment on attachment 8817411 [details]
Bug 1243415 - Improve screenshot unit tests and add tests for chrome scope.

https://reviewboard.mozilla.org/r/97690/#review98922

> Not sure to what this comment refers to. ScreenCapture? I wouldn't call it that way. Webdriver uses Screenshot and our client implementation too.

The chapter dealing with screen capture in WebDriver is named ‘Screen Capture’.  Please just keep the old names.
Merge conflicts prevented autolander from landing this patch series. Fixed it now.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e6091803e801
Capture methods should use the window as parameter. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/7ec63ef95514
Remove B2G related code from screenshot methods. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/1a07b3fcb30b
Make use of capture.js in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/62c28040b388
Add highlighting of elements for screenshots in chrome scope. r=ato,automatedtester
https://hg.mozilla.org/integration/autoland/rev/b7e547b2f592
Add hash format support for screenshots in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/f108db686ca2
Add support to take full screenshots in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/4bd98277bf9e
Check for a valid DOMWindow for screenshot in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/56ff773fe562
Improve screenshot unit tests and add tests for chrome scope. r=ato
backed out for wr tests failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8006311&repo=autoland
Flags: needinfo?(hskupin)
The problem as seen in the above test job are different hashes for the image and the reference. I'm fairly sure that this is because of the newly implemented scaling of images. I have to see how the reftests under webplatform tests are handling the reference image.

If there is no easy way to get this fixed we will have to revert the scaling logic.
Flags: needinfo?(hskupin)
Attached file screenshot (obsolete) — 
So examining such a test it looks like it is somewhat related to highlighting. There is a red line at the bottom of this image which is no present in the reference image.
Attachment #8819311 - Attachment filename: file_1243415.txt → file_1243415.png
Attachment #8819311 - Attachment mime type: text/plain → image/png
Attachment #8819311 - Attachment mime type: image/png → text/plain

        Attachment #8819311 -
        Attachment is obsolete: true

    
    

    
  
The issues with web-platform-tests appear when I use the ctx.DRAWWINDOW_DRAW_VIEW flag for drawing the window into the canvas. It basically means "Draw scrollbars and scroll the viewport if they are present."

With that included the screenshots are identical which would mean in a reftest that the test has to pass. What I don't understand in this case are the gray bars at the lower end of the screenshots. Those are no scrollbars nor other nodes, and I don't see it on my screen when running the tests locally.

James, can you please have a look at those screenshots and tell me what you think? If we don't want to make this change right now I'm happy to move the addition of DRAWWINDOW_DRAW_VIEW out to a follow-up bug.
Flags: needinfo?(james)

    
    

    
  
Btw when I enlarge the popup window where the tests are running in, I can still see this text but at the very right bottom corner. The popup doesn't have scrollbars, and as such we fail I think.

I wonder if the popup is opened correctly because I think it should have scrollbars. So the unexpected pass is clearly a failure!

    
    

    
  
Ok, so while investigating the failure in 2_tracks.html I noticed that at least for this test we set "overflow:hidden" on the html element. The popup window which runs all the tests has a size of 600x600 pixels, and as such the content we want to compare is getting drawn outside of the viewport. Due to the hidden value no scrollbars are drawn and as such drawWindow doesn't reach that area.

James it would be good to know why we do all that outside of the viewport.

    
    

    
  
We should not be doing anything outside the viewport; that's a test bug. In general we unfortunately have no good way to figure out which tests go outside the viewport (with anything relevant to the test). I filed <https://github.com/w3c/web-platform-tests/issues/4383>.

    
    

    
  
That's good to hear. Thank you Ms2ger. I would say given that this is an external dependency I will disable the drawing flag in content for now, and we can revisit once the webplatform tests follow the 600x600 window size.
Flags: needinfo?(james)

    
    

    
  
Removing the flag made all tests passing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c0c68bbbb02bbd213a756f5d27f86c919ef3f09

So I will better comment it out completely and we can add it again once the referenced web-platform-tests issue has been solved and tests are getting imported into mozilla-central.

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/11b5edf1c2b0
Capture methods should use the window as parameter. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/8f3e84d364e7
Remove B2G related code from screenshot methods. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/b775ed2eb015
Make use of capture.js in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/31e61178ca03
Add highlighting of elements for screenshots in chrome scope. r=ato,automatedtester
https://hg.mozilla.org/integration/autoland/rev/804ae2ac1d9b
Add hash format support for screenshots in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/a96fb5d278bf
Add support to take full screenshots in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/b8f4c98724eb
Check for a valid DOMWindow for screenshot in chrome scope. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/7702f475a614
Improve screenshot unit tests and add tests for chrome scope. r=ato

    
    

    
  
We would like to have this test-only patch for the next ESR release. Please uplift to aurora. Thanks.

          status-firefox52:
          --- → affected
Whiteboard: [checkin-needed-aurora]

    
    

    
  
needs rebasing 

grafting 381360:11b5edf1c2b0 "Bug 1243415 - Capture methods should use the window as parameter. r=automatedtester"
merging testing/marionette/capture.js
merging testing/marionette/listener.js
warning: conflicts while merging testing/marionette/capture.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(hskupin)

    
    

    
  
The problem here might be the missing patch on bug 1316975 for 52. I will check which parts need a fix.
Flags: needinfo?(hskupin)
Whiteboard: [checkin-needed-aurora]

    
    

    
  
The patch applies cleanly when bug 1316975 has been uplifted first. So please care about that for the test-only uplift to aurora. Thanks.
Whiteboard: [checkin-needed-aurora][uplift patch on bug 1316975 first]

    
  
Product: Testing → Remote Protocol

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: