Bug 1575511 - [marionette] Remove feature to highlight elements in screenshots. r=webdriver-reviewers,ato

The feature was used in the past to highlight broken elements for l10n
specific tests. Given that those tests don't exist anymore (for already
a long time) the highlight feature doesn't have to be kept alive.

Also it isn't covered by the WebDriver spec, and as such a custom feature
which is not worth keeping its code working. Especially with the Fission
work upcoming.

Differential Revision: https://phabricator.services.mozilla.com/D42823

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Henrik Skupin 2019-08-21 12:42:25 +00:00
parent 3983c191ec
commit c9b01104ed
5 changed files with 15 additions and 137 deletions

View File

@ -41,20 +41,15 @@ capture.Format = {
*
* @param {Node} node
* The node to take a screenshot of.
* @param {Array.<Node>=} highlights
* Optional array of nodes, around which a border will be marked to
* highlight them in the screenshot.
*
* @return {HTMLCanvasElement}
* The canvas element where the element has been painted on.
*/
capture.element = function(node, highlights = []) {
capture.element = function(node) {
let win = node.ownerGlobal;
let rect = node.getBoundingClientRect();
return capture.canvas(win, rect.left, rect.top, rect.width, rect.height, {
highlights,
});
return capture.canvas(win, rect.left, rect.top, rect.width, rect.height);
};
/**
@ -64,21 +59,17 @@ capture.element = function(node, highlights = []) {
* @param {DOMWindow} win
* The DOM window providing the document element to capture,
* and the offsets for the viewport.
* @param {Array.<Node>=} highlights
* Optional array of nodes, around which a border will be marked to
* highlight them in the screenshot.
*
* @return {HTMLCanvasElement}
* The canvas element where the viewport has been painted on.
*/
capture.viewport = function(win, highlights = []) {
capture.viewport = function(win) {
return capture.canvas(
win,
win.pageXOffset,
win.pageYOffset,
win.innerWidth,
win.innerHeight,
{ highlights }
win.innerHeight
);
};
@ -96,9 +87,6 @@ capture.viewport = function(win, highlights = []) {
* The width dimension of the rectangle to paint.
* @param {number} height
* The height dimension of the rectangle to paint.
* @param {Array.<Node>=} highlights
* Optional array of nodes, around which a border will be marked to
* highlight them in the screenshot.
* @param {HTMLCanvasElement=} canvas
* Optional canvas to reuse for the screenshot.
* @param {number=} flags
@ -115,7 +103,7 @@ capture.canvas = function(
top,
width,
height,
{ highlights = [], canvas = null, flags = null } = {}
{ canvas = null, flags = null } = {}
) {
const scale = win.devicePixelRatio;
@ -159,33 +147,10 @@ capture.canvas = function(
ctx.scale(scale, scale);
ctx.drawWindow(win, left, top, width, height, BG_COLOUR, flags);
if (highlights.length) {
ctx = capture.highlight_(ctx, highlights, top, left);
}
return canvas;
};
capture.highlight_ = function(context, highlights, top = 0, left = 0) {
if (typeof highlights == "undefined") {
throw new InvalidArgumentError("Missing highlights");
}
context.lineWidth = "2";
context.strokeStyle = "red";
context.save();
for (let el of highlights) {
let rect = el.getBoundingClientRect();
let oy = -top;
let ox = -left;
context.strokeRect(rect.left + ox, rect.top + oy, rect.width, rect.height);
}
return context;
};
/**
* Encode the contents of an HTMLCanvasElement to a Base64 encoded string.
*

View File

@ -1794,8 +1794,7 @@ class Marionette(object):
"""
return self._send_message("WebDriver:GetCookies")
def save_screenshot(self, fh, element=None, highlights=None,
full=True, scroll=True):
def save_screenshot(self, fh, element=None, full=True, scroll=True):
"""Takes a screenhot of a web element or the current frame and
saves it in the filehandle.
@ -1804,11 +1803,10 @@ class Marionette(object):
The rest of the parameters are defined like in screenshot()
"""
data = self.screenshot(element, highlights, "binary", full, scroll)
data = self.screenshot(element, "binary", full, scroll)
fh.write(data)
def screenshot(self, element=None, highlights=None, format="base64",
full=True, scroll=True):
def screenshot(self, element=None, format="base64", full=True, scroll=True):
"""Takes a screenshot of a web element or the current frame.
The screen capture is returned as a lossless PNG image encoded
@ -1820,10 +1818,6 @@ class Marionette(object):
:param element: The element to take a screenshot of. If None, will
take a screenshot of the current frame.
:param highlights: A list of
:class:`~marionette_driver.marionette.HTMLElement` objects to draw
a red box around in the returned screenshot.
:param format: if "base64" (the default), returns the screenshot
as a base64-string. If "binary", the data is decoded and
returned as raw binary. If "hash", the data is hashed using
@ -1840,12 +1834,8 @@ class Marionette(object):
if element:
element = element.id
lights = None
if highlights:
lights = [highlight.id for highlight in highlights]
body = {"id": element,
"highlights": lights,
"full": full,
"hash": False,
"scroll": scroll}

View File

@ -3006,8 +3006,6 @@ GeckoDriver.prototype.deleteSession = function() {
* @param {string=} id
* Optional web element reference to take a screenshot of.
* If undefined, a screenshot will be taken of the document element.
* @param {Array.<string>=} highlights
* List of web elements to highlight. Defaults to an empty list.
* @param {boolean=} full
* True to take a screenshot of the entire document element. Is only
* considered if <var>id</var> is not defined. Defaults to true.
@ -3024,10 +3022,9 @@ GeckoDriver.prototype.deleteSession = function() {
GeckoDriver.prototype.takeScreenshot = function(cmd) {
let win = assert.open(this.getCurrentWindow());
let { id, highlights, full, hash, scroll } = cmd.parameters;
let { id, full, hash, scroll } = cmd.parameters;
let format = hash ? capture.Format.Hash : capture.Format.Base64;
highlights = highlights || [];
full = typeof full == "undefined" ? true : full;
scroll = typeof scroll == "undefined" ? true : scroll;
@ -3036,10 +3033,6 @@ GeckoDriver.prototype.takeScreenshot = function(cmd) {
switch (this.context) {
case Context.Chrome:
let highlightEls = highlights
.map(ref => WebElement.fromUUID(ref, Context.Chrome))
.map(webEl => this.curBrowser.seenEls.get(webEl));
let canvas;
// element or full document element
@ -3052,11 +3045,11 @@ GeckoDriver.prototype.takeScreenshot = function(cmd) {
node = win.document.documentElement;
}
canvas = capture.element(node, highlightEls);
canvas = capture.element(node);
// viewport
} else {
canvas = capture.viewport(win, highlightEls);
canvas = capture.viewport(win);
}
switch (format) {
@ -3072,7 +3065,6 @@ GeckoDriver.prototype.takeScreenshot = function(cmd) {
return this.listener.takeScreenshot(format, {
id,
full,
highlights,
scroll,
});
}

View File

@ -14,7 +14,7 @@ import unittest
import urllib
from marionette_driver import By
from marionette_driver.errors import NoSuchElementException, NoSuchWindowException
from marionette_driver.errors import NoSuchWindowException
from marionette_harness import (
MarionetteTestCase,
skip,
@ -216,46 +216,6 @@ class TestScreenCaptureChrome(WindowManagerMixin, ScreenCaptureTestCase):
with self.assertRaises(ValueError):
self.marionette.screenshot(format="cheese")
@skip_if_mobile("Fennec doesn't support other chrome windows")
def test_highlight_elements(self):
dialog = self.open_dialog()
self.marionette.switch_to_window(dialog)
# Highlighting the element itself shouldn't make the image larger
element = self.marionette.find_element(By.ID, "test-list")
screenshot_element = self.marionette.screenshot(element=element)
screenshot_highlight = self.marionette.screenshot(element=element,
highlights=[element])
self.assertEqual(self.scale(self.get_element_dimensions(element)),
self.get_image_dimensions(screenshot_element))
self.assertNotEqual(screenshot_element, screenshot_highlight)
# Highlighting a sub element
button = self.marionette.find_element(By.ID, "choose-button")
screenshot_highlight_button = self.marionette.screenshot(element=element,
highlights=[button])
self.assertNotEqual(screenshot_element, screenshot_highlight_button)
self.assertNotEqual(screenshot_highlight, screenshot_highlight_button)
self.marionette.close_chrome_window()
self.marionette.switch_to_window(self.start_window)
def test_highlight_element_not_seen(self):
"""Check that for not found elements an exception is raised."""
with self.marionette.using_context('content'):
self.marionette.navigate(box)
content_element = self.marionette.find_element(By.ID, "green")
self.assertRaisesRegexp(NoSuchElementException, "Web element reference not seen before",
self.marionette.screenshot, highlights=[content_element])
chrome_document_element = self.document_element
with self.marionette.using_context('content'):
self.assertRaisesRegexp(NoSuchElementException,
"Web element reference not seen before",
self.marionette.screenshot,
highlights=[chrome_document_element])
class TestScreenCaptureContent(WindowManagerMixin, ScreenCaptureTestCase):
@ -362,25 +322,6 @@ class TestScreenCaptureContent(WindowManagerMixin, ScreenCaptureTestCase):
with self.assertRaises(ValueError):
self.marionette.screenshot(format="cheese")
def test_highlight_elements(self):
self.marionette.navigate(box)
element = self.marionette.find_element(By.TAG_NAME, "div")
# Highlighting the element itself shouldn't make the image larger
screenshot_element = self.marionette.screenshot(element=element)
screenshot_highlight = self.marionette.screenshot(element=element,
highlights=[element])
self.assertEqual(self.scale(self.get_element_dimensions(element)),
self.get_image_dimensions(screenshot_highlight))
self.assertNotEqual(screenshot_element, screenshot_highlight)
# Highlighting a sub element
paragraph = self.marionette.find_element(By.ID, "green")
screenshot_highlight_paragraph = self.marionette.screenshot(element=element,
highlights=[paragraph])
self.assertNotEqual(screenshot_element, screenshot_highlight_paragraph)
self.assertNotEqual(screenshot_highlight, screenshot_highlight_paragraph)
def test_save_screenshot(self):
expected = self.marionette.screenshot(format="binary")
with tempfile.TemporaryFile('w+b') as fh:

View File

@ -1636,9 +1636,6 @@ function switchToFrame(msg) {
* @param {boolean=} full
* True to take a screenshot of the entire document element. Is only
* considered if <var>id</var> is not defined. Defaults to true.
* @param {Array.<UUID>=} highlights
* Draw a border around the elements found by their web element
* references.
* @param {boolean=} scroll
* When <var>id</var> is given, scroll it into view before taking the
* screenshot. Defaults to true.
@ -1650,16 +1647,9 @@ function switchToFrame(msg) {
* @return {string}
* Base64 encoded string or a SHA-256 hash of the screenshot.
*/
function takeScreenshot(
format,
{ id, full = true, highlights = [], scroll = true } = {}
) {
function takeScreenshot(format, { id, full = true, scroll = true } = {}) {
let win = curContainer.frame;
let highlightEls = highlights
.map(ref => WebElement.fromUUID(ref, "content"))
.map(webEl => seenEls.get(webEl, win));
let canvas;
// element or full document element
@ -1675,11 +1665,11 @@ function takeScreenshot(
el = win.document.documentElement;
}
canvas = capture.element(el, highlightEls);
canvas = capture.element(el);
// viewport
} else {
canvas = capture.viewport(win, highlightEls);
canvas = capture.viewport(win);
}
switch (format) {