Don't display grid adorners on shadow roots

If an element in the Elements tree is a grid, it will get a grid adorner
but there's a bug where if that element contains a shadow root, then it
will also be getting the same adorner.

To avoid this, we skip document fragment nodes in the code.

Before: https://imgur.com/YU041Po.jpg
After: https://imgur.com/qaFdQle.jpg

Bug: 1129760
Change-Id: Ice4d9044b2a8a77a20d1db05ba4fb2acaa17775c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2418171
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
This commit is contained in:
Patrick Brosset
2020-09-24 08:49:55 +00:00
committed by Commit Bot
parent eeeaa840fa
commit a3fd937cbd
6 changed files with 65 additions and 33 deletions
+2 -1
View File
@@ -1951,7 +1951,8 @@ export class ElementsTreeElement extends UI.TreeOutline.TreeElement {
const node = this.node();
const nodeId = node.id;
if (node.nodeType() === Node.COMMENT_NODE || nodeId === undefined) {
if (node.nodeType() === Node.COMMENT_NODE || node.nodeType() === Node.DOCUMENT_FRAGMENT_NODE ||
nodeId === undefined) {
return;
}
+17 -9
View File
@@ -4,7 +4,7 @@
import {click, enableExperiment, goToResource} from '../../shared/helper.js';
import {describe, it} from '../../shared/mocha-extensions.js';
import {assertActiveAdorners, assertInactiveAdorners, expandSelectedNodeRecursively, INACTIVE_GRID_ADORNER_SELECTOR, waitForContentOfSelectedElementsNode, waitForElementsStyleSection} from '../helpers/elements-helpers.js';
import {expandSelectedNodeRecursively, INACTIVE_GRID_ADORNER_SELECTOR, waitForAdorners, waitForContentOfSelectedElementsNode, waitForElementsStyleSection} from '../helpers/elements-helpers.js';
const prepareElementsTab = async () => {
await waitForElementsStyleSection();
@@ -18,20 +18,28 @@ describe('Adornment in the Elements Tab', async () => {
await enableExperiment('cssGridFeatures');
await prepareElementsTab();
await assertInactiveAdorners([
'grid',
'grid',
await waitForAdorners([
{textContent: 'grid', isActive: false},
{textContent: 'grid', isActive: false},
]);
await assertActiveAdorners([]);
// Toggle both grid adorners on and try to select them with the active selector
await click(INACTIVE_GRID_ADORNER_SELECTOR);
await click(INACTIVE_GRID_ADORNER_SELECTOR);
await assertInactiveAdorners([]);
await assertActiveAdorners([
'grid',
'grid',
await waitForAdorners([
{textContent: 'grid', isActive: true},
{textContent: 'grid', isActive: true},
]);
});
it('does not display adorners on shadow roots when their parents are grids', async () => {
await goToResource('elements/adornment-shadow.html');
await enableExperiment('cssGridFeatures');
await prepareElementsTab();
await waitForAdorners([
{textContent: 'grid', isActive: false},
]);
});
});
+5 -7
View File
@@ -4,7 +4,7 @@
import {enableExperiment, goToResource, step} from '../../shared/helper.js';
import {describe, it} from '../../shared/mocha-extensions.js';
import {assertActiveAdorners, assertInactiveAdorners, expandSelectedNodeRecursively, openLayoutPane, toggleElementCheckboxInLayoutPane, waitForContentOfSelectedElementsNode, waitForElementsStyleSection} from '../helpers/elements-helpers.js';
import {expandSelectedNodeRecursively, openLayoutPane, toggleElementCheckboxInLayoutPane, waitForAdorners, waitForContentOfSelectedElementsNode, waitForElementsStyleSection} from '../helpers/elements-helpers.js';
describe('Layout Pane in the Elements Tab', async function() {
it('displays Layout pane', async () => {
@@ -15,15 +15,13 @@ describe('Layout Pane in the Elements Tab', async function() {
await waitForContentOfSelectedElementsNode('<body>\u200B');
await expandSelectedNodeRecursively();
});
await assertInactiveAdorners([
'grid',
await waitForAdorners([
{textContent: 'grid', isActive: false},
]);
await assertActiveAdorners([]);
await openLayoutPane();
await toggleElementCheckboxInLayoutPane();
await assertInactiveAdorners([]);
await assertActiveAdorners([
'grid',
await waitForAdorners([
{textContent: 'grid', isActive: true},
]);
});
});
+24 -16
View File
@@ -21,6 +21,7 @@ const CLS_PANE_SELECTOR = '.styles-sidebar-toolbar-pane';
const CLS_BUTTON_SELECTOR = '[aria-label="Element Classes"]';
const CLS_INPUT_SELECTOR = '[aria-placeholder="Add new class"]';
const LAYOUT_PANE_TAB_SELECTOR = '[aria-label="Layout"]';
const ADORNER_SELECTOR = 'devtools-adorner';
export const INACTIVE_GRID_ADORNER_SELECTOR = '[aria-label="Enable grid mode"]';
export const ACTIVE_GRID_ADORNER_SELECTOR = '[aria-label="Disable grid mode"]';
const ELEMENT_CHECKBOX_IN_LAYOUT_PANE_SELECTOR = '.elements input[type=checkbox]';
@@ -32,23 +33,30 @@ export const openLayoutPane = async () => {
});
};
export const assertInactiveAdorners = async (expectedAdorners: string[]) => {
await step('Assert inactive adorners in Elements panel', async () => {
const actualAdorners = await $$(INACTIVE_GRID_ADORNER_SELECTOR);
const actualAdornersContent = await Promise.all(actualAdorners.map(n => n.evaluate(node => node.textContent)));
assert.deepEqual(
actualAdornersContent, expectedAdorners,
`did not have exactly ${expectedAdorners.length} adorner(s) in the inactive state`);
});
};
export const waitForAdorners = async (expectedAdorners: {textContent: string, isActive: boolean}[]) => {
await waitForFunction(async () => {
const actualAdorners = await $$(ADORNER_SELECTOR);
const actualAdornersStates = await Promise.all(actualAdorners.map(n => {
return n.evaluate((node, activeSelector: string) => {
return {textContent: node.textContent, isActive: node.matches(activeSelector)};
}, ACTIVE_GRID_ADORNER_SELECTOR);
}));
export const assertActiveAdorners = async (expectedAdorners: string[]) => {
await step('Assert active adorners in Elements panel', async () => {
const actualAdorners = await $$(ACTIVE_GRID_ADORNER_SELECTOR);
const actualAdornersContent = await Promise.all(actualAdorners.map(n => n.evaluate(node => node.textContent)));
assert.deepEqual(
actualAdornersContent, expectedAdorners,
`did not have exactly ${expectedAdorners.length} adorner(s) in the inactive state`);
if (actualAdornersStates.length !== expectedAdorners.length) {
return false;
}
for (let i = 0; i < actualAdornersStates.length; i++) {
const index = expectedAdorners.findIndex(expected => {
const actual = actualAdornersStates[i];
return expected.textContent === actual.textContent && expected.isActive === actual.isActive;
});
if (index !== -1) {
expectedAdorners.splice(index, 1);
}
}
return expectedAdorners.length === 0;
});
};
+1
View File
@@ -6,6 +6,7 @@ import("../../../../scripts/build/ninja/copy.gni")
copy_to_gen("elements") {
sources = [
"adornment-shadow.html",
"adornment.html",
"css-grid.html",
"css-variables.html",
@@ -0,0 +1,16 @@
<!--
Copyright 2020 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->
<!DOCTYPE html>
<style>
.grid {
display: inline-grid;
}
</style>
<div class="grid"></div>
<script>
const el = document.querySelector(".grid");
const shadowRoot = el.attachShadow({mode: 'open'});
</script>