Skip to content

GitLab

T
tangled
Commit 8188223e authored by Zoe Clifford's avatar Zoe Clifford Committed by Commit Bot
Browse files
Options

Fix a bounding box inconsistency in DOMSnapshot.getSnapshot. 

Element.BoundsInViewport was applying a viewport transformation that
layout_object->AbsoluteBoundingBoxRect was not. This lead to weird
inconsistencies in the LayoutTreeNode boundingBox output, such as text
in a div with a boundingBox that was completely outside the div's
boundingBox.

This fix just skips calling BoundsInViewport and instead goes down the
AbsoluteBoundingBoxRect path for every node.

Bug: 
Change-Id: I60f7c166658530ee6b57489c1c81fab21ea038fb
Reviewed-on: https://chromium-review.googlesource.com/736713
Commit-Queue: Zoe Clifford <zoeclifford@chromium.org>
Reviewed-by: default avatarPavel Feldman <pfeldman@chromium.org>
Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512885}
parent 054e29c3
Hide whitespace changes
Inline Side-by-side
Showing with 320 additions and 23 deletions
headless/lib/dom_tree_extraction_expected_nodes.txt
View file @ 8188223e
......@@ -237,8 +237,8 @@
"boundingBox": {
"height": 200.0,
"width": 400.0,
"x": 10.0,
"y": 63.0
"x": 0.0,
"y": 0.0
},
"childNodeIndexes": [ 20, 21 ],
"frameId": "?",
......@@ -259,8 +259,8 @@
"boundingBox": {
"height": 171.0,
"width": 384.0,
"x": 18.0,
"y": 71.0
"x": 8.0,
"y": 8.0
},
"childNodeIndexes": [ 22, 23, 25 ],
"layoutNodeIndex": 9,
......@@ -280,8 +280,8 @@
"boundingBox": {
"height": 37.0,
"width": 384.0,
"x": 18.0,
"y": 71.0
"x": 8.0,
"y": 8.0
},
"childNodeIndexes": [ 24 ],
"layoutNodeIndex": 10,
......@@ -520,10 +520,10 @@
{
"backendNodeId": 42,
"boundingBox": {
"height": 0.0,
"height": 17.0,
"width": 0.0,
"x": 0.0,
"y": 0.0
"x": 8.0,
"y": 329.0
},
"inlineTextNodes": [ {
"boundingBox": {
......
third_party/WebKit/LayoutTests/inspector-protocol/dom-snapshot/dom-snapshot-getSnapshot-expected.txt
View file @ 8188223e
......@@ -1219,8 +1219,8 @@ Tests DOMSnapshot.getSnapshot method.
{
"domNodeIndex": 35,
"boundingBox": {
"x": 10,
"y": 634,
"x": 0,
"y": 0,
"width": 400,
"height": 200
},
......@@ -1229,8 +1229,8 @@ Tests DOMSnapshot.getSnapshot method.
{
"domNodeIndex": 42,
"boundingBox": {
"x": 18,
"y": 642,
"x": 8,
"y": 8,
"width": 384,
"height": 184
},
......@@ -1239,8 +1239,8 @@ Tests DOMSnapshot.getSnapshot method.
{
"domNodeIndex": 44,
"boundingBox": {
"x": 18,
"y": 642,
"x": 8,
"y": 8,
"width": 384,
"height": 16
},
......
third_party/WebKit/LayoutTests/inspector-protocol/dom-snapshot/dom-snapshot-getSnapshot-viewport-expected.txt 0 → 100644
View file @ 8188223e
Tests DOMSnapshot.getSnapshot method on a mobile page.
Emulating device: 600x600x1
{
"domNodes": [
{
"nodeType": 9,
"nodeName": "#document",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
1,
2,
3
],
"layoutNodeIndex": 0,
"documentURL": "<string>",
"baseURL": "<string>",
"documentEncoding": "windows-1252",
"frameId": "<string>"
},
{
"nodeType": 10,
"nodeName": "html",
"nodeValue": "",
"backendNodeId": "<number>",
"publicId": "",
"systemId": ""
},
{
"nodeType": 8,
"nodeName": "#comment",
"nodeValue": " domSnapshot test for a document with a viewport. ",
"backendNodeId": "<number>"
},
{
"nodeType": 1,
"nodeName": "HTML",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
4,
15
],
"layoutNodeIndex": 1,
"frameId": "<string>"
},
{
"nodeType": 1,
"nodeName": "HEAD",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
5,
6,
7,
9,
10,
12,
13,
14
]
},
{
"nodeType": 1,
"nodeName": "META",
"nodeValue": "",
"backendNodeId": "<number>",
"attributes": [
{
"name": "name",
"value": "viewport"
},
{
"name": "content",
"value": "width=300"
}
]
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n",
"backendNodeId": "<number>"
},
{
"nodeType": 1,
"nodeName": "STYLE",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
8
]
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n@font-face {\n font-family: 'ahem';\n src: url(../../resources/Ahem.ttf);\n}\n",
"backendNodeId": "<number>"
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n",
"backendNodeId": "<number>"
},
{
"nodeType": 1,
"nodeName": "STYLE",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
11
]
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n* {\n margin: 0;\n padding: 0;\n}\ndiv {\n position: absolute;\n top: 100px;\n left: 100px;\n width: 100px;\n height: 100px;\n background-color: green;\n}\n",
"backendNodeId": "<number>"
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n",
"backendNodeId": "<number>"
},
{
"nodeType": 8,
"nodeName": "#comment",
"nodeValue": " The div's box should have size and position of 100x100 to match the\n computed style, even though it's visually scaled by the viewport. The\n text's box should also be inside the div's box, not outside it. ",
"backendNodeId": "<number>"
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n",
"backendNodeId": "<number>"
},
{
"nodeType": 1,
"nodeName": "BODY",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
16,
18
],
"layoutNodeIndex": 2
},
{
"nodeType": 1,
"nodeName": "DIV",
"nodeValue": "",
"backendNodeId": "<number>",
"childNodeIndexes": [
17
],
"attributes": [
{
"name": "style",
"value": "font-family: ahem;"
},
{
"name": "id",
"value": "100x100"
}
],
"layoutNodeIndex": 3
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "Boxes!",
"backendNodeId": "<number>",
"layoutNodeIndex": 4
},
{
"nodeType": 3,
"nodeName": "#text",
"nodeValue": "\n\n",
"backendNodeId": "<number>"
}
],
"layoutTreeNodes": [
{
"domNodeIndex": 0,
"boundingBox": {
"x": 0,
"y": 0,
"width": 300,
"height": 300
}
},
{
"domNodeIndex": 3,
"boundingBox": {
"x": 0,
"y": 0,
"width": 300,
"height": 0
}
},
{
"domNodeIndex": 15,
"boundingBox": {
"x": 0,
"y": 0,
"width": 300,
"height": 0
}
},
{
"domNodeIndex": 16,
"boundingBox": {
"x": 100,
"y": 100,
"width": 100,
"height": 100
}
},
{
"domNodeIndex": 17,
"boundingBox": {
"x": 100,
"y": 100,
"width": 96,
"height": 16
},
"layoutText": "Boxes!",
"inlineTextNodes": [
{
"boundingBox": {
"x": 100,
"y": 100,
"width": 96,
"height": 16
},
"startCharacterIndex": 0,
"numCharacters": 6
}
]
}
],
"computedStyles": []
}
third_party/WebKit/LayoutTests/inspector-protocol/dom-snapshot/dom-snapshot-getSnapshot-viewport.js 0 → 100644
View file @ 8188223e
(async function(testRunner) {
var {page, session, dp} = await testRunner.startBlank('Tests DOMSnapshot.getSnapshot method on a mobile page.');
var DeviceEmulator = await testRunner.loadScript('../resources/device-emulator.js');
var deviceEmulator = new DeviceEmulator(testRunner, session);
await deviceEmulator.emulate(600, 600, 1);
// The viewport width is 300px, half the device width.
await session.navigate('../resources/dom-snapshot-viewport.html');
function stabilize(key, value) {
var unstableKeys = ['documentURL', 'baseURL', 'frameId', 'backendNodeId'];
if (unstableKeys.indexOf(key) !== -1)
return '<' + typeof(value) + '>';
if (typeof value === 'string' && value.indexOf('/dom-snapshot/') !== -1)
value = '<value>';
return value;
}
var whitelist = [];
var response = await dp.DOMSnapshot.getSnapshot({'computedStyleWhitelist': whitelist});
if (response.error)
testRunner.log(response);
else
testRunner.log(JSON.stringify(response.result, stabilize, 2));
testRunner.completeTest();
})
third_party/WebKit/LayoutTests/inspector-protocol/resources/dom-snapshot-viewport.html 0 → 100644
View file @ 8188223e
<!DOCTYPE html>
<!-- domSnapshot test for a document with a viewport. -->
<meta name=viewport content='width=300'>
<style>
@font-face {
font-family: 'ahem';
src: url(../../resources/Ahem.ttf);
}
</style>
<style>
* {
margin: 0;
padding: 0;
}
div {
position: absolute;
top: 100px;
left: 100px;
width: 100px;
height: 100px;
background-color: green;
}
</style>
<!-- The div's box should have size and position of 100x100 to match the
computed style, even though it's visually scaled by the viewport. The
text's box should also be inside the div's box, not outside it. -->
<div style='font-family: ahem;' id='100x100'>Boxes!</div>
third_party/WebKit/Source/core/inspector/InspectorDOMSnapshotAgent.cpp
View file @ 8188223e
......@@ -309,14 +309,11 @@ int InspectorDOMSnapshotAgent::VisitLayoutTreeNode(Node* node, int node_index) {
if (!layout_object)
return -1;
auto layout_tree_node =
protocol::DOMSnapshot::LayoutTreeNode::create()
.setDomNodeIndex(node_index)
.setBoundingBox(BuildRectForFloatRect(
node->IsElementNode()
? FloatRect(ToElement(node)->BoundsInViewport())
: layout_object->AbsoluteBoundingBoxRect()))
.build();
auto layout_tree_node = protocol::DOMSnapshot::LayoutTreeNode::create()
.setDomNodeIndex(node_index)
.setBoundingBox(BuildRectForFloatRect(
layout_object->AbsoluteBoundingBoxRect()))
.build();
int style_index = GetStyleIndexForNode(node);
if (style_index != -1)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment