diff --git a/devtools/client/webconsole/components/Output/ConsoleTable.js b/devtools/client/webconsole/components/Output/ConsoleTable.js index ad948ec339f0..87ea22ba0c6e 100644 --- a/devtools/client/webconsole/components/Output/ConsoleTable.js +++ b/devtools/client/webconsole/components/Output/ConsoleTable.js @@ -12,6 +12,7 @@ const actions = require("devtools/client/webconsole/actions/messages"); const { l10n, getArrayTypeNames, + getDescriptorValue, } = require("devtools/client/webconsole/utils/messages"); loader.lazyGetter(this, "MODE", function() { return require("devtools/client/shared/components/reps/reps").MODE; @@ -37,6 +38,7 @@ class ConsoleTable extends Component { parameters: PropTypes.array.isRequired, serviceContainer: PropTypes.object.isRequired, id: PropTypes.string.isRequired, + // Remove in Firefox 74. See Bug 1597955. tableData: PropTypes.object, }; } @@ -47,10 +49,13 @@ class ConsoleTable extends Component { this.getRows = this.getRows.bind(this); } + // Remove in Firefox 74. See Bug 1597955. componentWillMount() { const { id, dispatch, parameters, tableData } = this.props; - if (!Array.isArray(parameters) || parameters.length === 0 || tableData) { + const data = tableData || (parameters[0] && parameters[0].ownProperties); + + if (!Array.isArray(parameters) || parameters.length === 0 || data) { return; } @@ -110,20 +115,27 @@ class ConsoleTable extends Component { render() { const { parameters, tableData } = this.props; - const headersGrip = parameters[1]; + const [valueGrip, headersGrip] = parameters; const headers = headersGrip && headersGrip.preview ? headersGrip.preview.items : null; - // if tableData is nullable, we don't show anything. - if (!tableData) { + const data = + valueGrip && valueGrip.ownProperties + ? valueGrip.ownProperties + : tableData; + + // if we don't have any data, don't show anything. + if (!data) { return null; } - const { columns, items } = getTableItems( - tableData, - getParametersDataType(parameters), - headers - ); + const dataType = getParametersDataType(parameters); + + // Remove deprecatedGetTableItems in Firefox 74. See Bug 1597955. + const { columns, items } = + valueGrip && valueGrip.ownProperties + ? getTableItems(data, dataType, headers) + : deprecatedGetTableItems(data, dataType, headers); return dom.div( { @@ -146,10 +158,11 @@ function getParametersDataType(parameters = null) { return parameters[0].class; } -function getTableItems(data = {}, type, headers = null) { - const INDEX_NAME = "_index"; - const VALUE_NAME = "_value"; - const namedIndexes = { +const INDEX_NAME = "_index"; +const VALUE_NAME = "_value"; + +function getNamedIndexes(type) { + return { [INDEX_NAME]: getArrayTypeNames() .concat("Object") .includes(type) @@ -158,6 +171,19 @@ function getTableItems(data = {}, type, headers = null) { [VALUE_NAME]: l10n.getStr("table.value"), key: l10n.getStr("table.key"), }; +} + +function hasValidCustomHeaders(headers) { + return ( + Array.isArray(headers) && + headers.every( + header => typeof header === "string" || Number.isInteger(Number(header)) + ) + ); +} + +function getTableItems(data = {}, type, headers = null) { + const namedIndexes = getNamedIndexes(type); let columns = new Map(); const items = []; @@ -167,11 +193,7 @@ function getTableItems(data = {}, type, headers = null) { Object.keys(item).forEach(key => addColumn(key)); }; - const hasValidCustomHeaders = - Array.isArray(headers) && - headers.every( - header => typeof header === "string" || Number.isInteger(Number(header)) - ); + const validCustomHeaders = hasValidCustomHeaders(headers); const addColumn = function(columnIndex) { const columnExists = columns.has(columnIndex); @@ -180,7 +202,99 @@ function getTableItems(data = {}, type, headers = null) { if ( !columnExists && !hasMaxColumns && - (!hasValidCustomHeaders || + (!validCustomHeaders || + headers.includes(columnIndex) || + columnIndex === INDEX_NAME) + ) { + columns.set(columnIndex, namedIndexes[columnIndex] || columnIndex); + } + }; + + for (let [index, property] of Object.entries(data)) { + if (type !== "Object" && index == parseInt(index, 10)) { + index = parseInt(index, 10); + } + + const item = { + [INDEX_NAME]: index, + }; + + const propertyValue = getDescriptorValue(property); + + if (propertyValue && propertyValue.ownProperties) { + const entries = propertyValue.ownProperties; + for (const [key, entry] of Object.entries(entries)) { + item[key] = getDescriptorValue(entry); + } + } else if ( + propertyValue && + propertyValue.preview && + (type === "Map" || type === "WeakMap") + ) { + item.key = propertyValue.preview.key; + item[VALUE_NAME] = propertyValue.preview.value; + } else { + item[VALUE_NAME] = propertyValue; + } + + addItem(item); + + if (items.length === TABLE_ROW_MAX_ITEMS) { + break; + } + } + + // Some headers might not be present in the items, so we make sure to + // return all the headers set by the user. + if (validCustomHeaders) { + headers.forEach(header => addColumn(header)); + } + + // We want to always have the index column first + if (columns.has(INDEX_NAME)) { + const index = columns.get(INDEX_NAME); + columns.delete(INDEX_NAME); + columns = new Map([[INDEX_NAME, index], ...columns.entries()]); + } + + // We want to always have the values column last + if (columns.has(VALUE_NAME)) { + const index = columns.get(VALUE_NAME); + columns.delete(VALUE_NAME); + columns.set(VALUE_NAME, index); + } + + return { + columns, + items, + }; +} + +/** + * This function is handling console.table packets from pre-Firefox72 servers. + * Remove in Firefox 74. See Bug 1597955. + */ +function deprecatedGetTableItems(data = {}, type, headers = null) { + const namedIndexes = getNamedIndexes(type); + + let columns = new Map(); + const items = []; + + const addItem = function(item) { + items.push(item); + Object.keys(item).forEach(key => addColumn(key)); + }; + + const validCustomHeaders = hasValidCustomHeaders(headers); + + const addColumn = function(columnIndex) { + const columnExists = columns.has(columnIndex); + const hasMaxColumns = columns.size == TABLE_COLUMN_MAX_ITEMS; + + if ( + !columnExists && + !hasMaxColumns && + (!validCustomHeaders || headers.includes(columnIndex) || columnIndex === INDEX_NAME) ) { @@ -234,7 +348,7 @@ function getTableItems(data = {}, type, headers = null) { // Some headers might not be present in the items, so we make sure to // return all the headers set by the user. - if (hasValidCustomHeaders) { + if (validCustomHeaders) { headers.forEach(header => addColumn(header)); } diff --git a/devtools/client/webconsole/reducers/messages.js b/devtools/client/webconsole/reducers/messages.js index d721c7feddfc..75a2a667839e 100644 --- a/devtools/client/webconsole/reducers/messages.js +++ b/devtools/client/webconsole/reducers/messages.js @@ -47,6 +47,12 @@ loader.lazyRequireGetter( "devtools/client/webconsole/utils/messages", true ); +loader.lazyRequireGetter( + this, + "getDescriptorValue", + "devtools/client/webconsole/utils/messages", + true +); loader.lazyRequireGetter( this, "getParentWarningGroupMessageId", @@ -1364,6 +1370,18 @@ function isTextInParameter(text, regex, parameter) { } } + if (parameter && parameter.ownProperties) { + for (const [key, desc] of Object.entries(parameter.ownProperties)) { + if (matchStr(key)) { + return true; + } + + if (isTextInParameter(text, regex, getDescriptorValue(desc))) { + return true; + } + } + } + return false; } diff --git a/devtools/client/webconsole/test/browser/browser.ini b/devtools/client/webconsole/test/browser/browser.ini index f9551b2773d7..bbe6efa7215e 100644 --- a/devtools/client/webconsole/test/browser/browser.ini +++ b/devtools/client/webconsole/test/browser/browser.ini @@ -326,6 +326,7 @@ skip-if = true # Bug XXX # Bug 1405250 [browser_webconsole_console_group.js] [browser_webconsole_console_logging_workers_api.js] skip-if = e10s # SharedWorkers console events are not received on the current process because they could run on any process. +[browser_webconsole_console_table_post_alterations.js] [browser_webconsole_console_table.js] [browser_webconsole_console_timeStamp.js] [browser_webconsole_console_trace_distinct.js] diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_console_table.js b/devtools/client/webconsole/test/browser/browser_webconsole_console_table.js index 1dbffd46d891..6ccc73fde23b 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_console_table.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_console_table.js @@ -258,7 +258,7 @@ add_task(async function() { input: [{ a: { b: 34 } }], expected: { columns: ["(index)", "a"], - rows: [["0", "Object { … }"]], + rows: [["0", "Object { b: 34 }"]], }, additionalTest: async function(node) { info("Check that object in a cell can be expanded"); @@ -312,7 +312,7 @@ async function testItem(testCase, node) { is( JSON.stringify(testCase.expected.columns), JSON.stringify(columns.map(column => column.textContent)), - "table has the expected columns" + `${testCase.info} | table has the expected columns` ); // We don't really have rows since we are using a CSS grid in order to have a sticky @@ -321,14 +321,18 @@ async function testItem(testCase, node) { is( testCase.expected.rows.length, cells.length / columnsNumber, - "table has the expected number of rows" + `${testCase.info} | table has the expected number of rows` ); testCase.expected.rows.forEach((expectedRow, rowIndex) => { const startIndex = rowIndex * columnsNumber; // Slicing the cells array so we can get the current "row". const rowCells = cells.slice(startIndex, startIndex + columnsNumber); - is(rowCells.map(x => x.textContent).join(" | "), expectedRow.join(" | ")); + is( + rowCells.map(x => x.textContent).join(" | "), + expectedRow.join(" | "), + `${testCase.info} | row has the expected content` + ); }); if (testCase.expected.overflow) { diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_console_table_post_alterations.js b/devtools/client/webconsole/test/browser/browser_webconsole_console_table_post_alterations.js new file mode 100644 index 000000000000..95394753e3ed --- /dev/null +++ b/devtools/client/webconsole/test/browser/browser_webconsole_console_table_post_alterations.js @@ -0,0 +1,63 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Check that calling console.table on a variable which is modified after the +// console.table call only shows data for when the variable was logged. + +const TEST_URI = `data:text/html,Test console.table with modified variable`; + +add_task(async function() { + const hud = await openNewTabAndConsole(TEST_URI); + + await ContentTask.spawn(gBrowser.selectedBrowser, null, () => { + const x = ["a", "b"]; + content.wrappedJSObject.console.table(x); + x.push("c"); + content.wrappedJSObject.console.table(x); + x.sort((a, b) => b - a); + content.wrappedJSObject.console.table(x); + }); + + const [table1, table2, table3] = await waitFor(() => { + const res = hud.ui.outputNode.querySelectorAll( + ".message .new-consoletable" + ); + if (res.length === 3) { + return res; + } + return null; + }); + + info("Check the rows of the first table"); + checkTable(table1, [[0, "a"], [1, "b"]]); + + info("Check the rows of the table after adding an element to the array"); + checkTable(table2, [[0, "a"], [1, "b"], [2, "c"]]); + + info("Check the rows of the table after sorting the array"); + checkTable(table3, [[0, "c"], [1, "b"], [2, "a"]]); +}); + +function checkTable(node, expectedRows) { + const columns = Array.from(node.querySelectorAll("[role=columnheader]")); + const columnsNumber = columns.length; + const cells = Array.from(node.querySelectorAll("[role=gridcell]")); + + // We don't really have rows since we are using a CSS grid in order to have a sticky + // header on the table. So we check the "rows" by dividing the number of cells by the + // number of columns. + is( + cells.length / columnsNumber, + expectedRows.length, + "table has the expected number of rows" + ); + + expectedRows.forEach((expectedRow, rowIndex) => { + const startIndex = rowIndex * columnsNumber; + // Slicing the cells array so we can get the current "row". + const rowCells = cells.slice(startIndex, startIndex + columnsNumber); + is(rowCells.map(x => x.textContent).join(" | "), expectedRow.join(" | ")); + }); +} diff --git a/devtools/client/webconsole/test/node/fixtures/stubs/consoleApi.js b/devtools/client/webconsole/test/node/fixtures/stubs/consoleApi.js index e1b893656d62..d64efe855f08 100644 --- a/devtools/client/webconsole/test/node/fixtures/stubs/consoleApi.js +++ b/devtools/client/webconsole/test/node/fixtures/stubs/consoleApi.js @@ -458,14 +458,26 @@ stubPackets.set(`console.table(['red', 'green', 'blue']);`, { "extensible": true, "frozen": false, "sealed": false, - "preview": { - "kind": "ArrayLike", - "length": 3, - "items": [ - "red", - "green", - "blue" - ] + "preview": null, + "ownProperties": { + "0": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "red" + }, + "1": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "green" + }, + "2": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "blue" + } } } ], @@ -1150,14 +1162,26 @@ stubPackets.set(`console.table(['a', 'b', 'c'])`, { "extensible": true, "frozen": false, "sealed": false, - "preview": { - "kind": "ArrayLike", - "length": 3, - "items": [ - "a", - "b", - "c" - ] + "preview": null, + "ownProperties": { + "0": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "a" + }, + "1": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "b" + }, + "2": { + "configurable": true, + "enumerable": true, + "writable": true, + "value": "c" + } } } ], diff --git a/devtools/client/webconsole/utils/messages.js b/devtools/client/webconsole/utils/messages.js index 664f217a3cde..a193051a7504 100644 --- a/devtools/client/webconsole/utils/messages.js +++ b/devtools/client/webconsole/utils/messages.js @@ -689,9 +689,25 @@ function getArrayTypeNames() { ]; } +function getDescriptorValue(descriptor) { + if (!descriptor) { + return descriptor; + } + + if (Object.prototype.hasOwnProperty.call(descriptor, "safeGetterValues")) { + return descriptor.safeGetterValues; + } + + if (Object.prototype.hasOwnProperty.call(descriptor, "value")) { + return descriptor.value; + } + return descriptor; +} + module.exports = { createWarningGroupMessage, getArrayTypeNames, + getDescriptorValue, getInitialMessageCountForViewport, getParentWarningGroupMessageId, getWarningGroupType, diff --git a/devtools/server/actors/webconsole.js b/devtools/server/actors/webconsole.js index ee42d82c7bb9..961c2430270c 100644 --- a/devtools/server/actors/webconsole.js +++ b/devtools/server/actors/webconsole.js @@ -17,6 +17,7 @@ const { ObjectActor } = require("devtools/server/actors/object"); const { LongStringActor } = require("devtools/server/actors/string"); const { createValueGrip, + isArray, stringIsLong, } = require("devtools/server/actors/object/utils"); const DevToolsUtils = require("devtools/shared/DevToolsUtils"); @@ -2020,7 +2021,6 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, { const result = WebConsoleUtils.cloneObject(message); result.workerType = WebConsoleUtils.getWorkerType(result) || "none"; - result.sourceId = this.getActorIdForInternalSourceId(result.sourceId); delete result.wrappedJSObject; @@ -2046,12 +2046,94 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, { return this.createValueGrip(string); }); + if (result.level === "table") { + const tableItems = this._getConsoleTableMessageItems(result); + if (tableItems) { + result.arguments[0].ownProperties = tableItems; + result.arguments[0].preview = null; + } + + // Only return the 2 first params. + result.arguments = result.arguments.slice(0, 2); + } + result.category = message.category || "webdev"; result.innerWindowID = message.innerID; return result; }, + /** + * Return the properties needed to display the appropriate table for a given + * console.table call. + * This function does a little more than creating an ObjectActor for the first + * parameter of the message. When layout out the console table in the output, we want + * to be able to look into sub-properties so the table can have a different layout ( + * for arrays of arrays, objects with objects properties, arrays of objects, …). + * So here we need to retrieve the properties of the first parameter, and also all the + * sub-properties we might need. + * + * @param {Object} result: The console.table message. + * @returns {Object} An object containing the properties of the first argument of the + * console.table call. + */ + _getConsoleTableMessageItems: function(result) { + if ( + !result || + !Array.isArray(result.arguments) || + result.arguments.length == 0 + ) { + return null; + } + + const [tableItemGrip] = result.arguments; + const dataType = tableItemGrip.class; + const needEntries = ["Map", "WeakMap", "Set", "WeakSet"].includes(dataType); + const ignoreNonIndexedProperties = isArray(tableItemGrip); + + const tableItemActor = this.getActorByID(tableItemGrip.actor); + if (!tableItemActor) { + return null; + } + + // Retrieve the properties (or entries for Set/Map) of the console table first arg. + const iterator = needEntries + ? tableItemActor.enumEntries() + : tableItemActor.enumProperties({ + ignoreNonIndexedProperties, + }); + const { ownProperties } = iterator.all(); + + // The iterator returns a descriptor for each property, wherein the value could be + // in one of those sub-property. + const descriptorKeys = ["safeGetterValues", "getterValue", "value"]; + + Object.values(ownProperties).forEach(desc => { + if (typeof desc !== "undefined") { + descriptorKeys.forEach(key => { + if (desc && desc.hasOwnProperty(key)) { + const grip = desc[key]; + + // We need to load sub-properties as well to render the table in a nice way. + const actor = grip && this.getActorByID(grip.actor); + if (actor) { + const res = actor + .enumProperties({ + ignoreNonIndexedProperties: isArray(grip), + }) + .all(); + if (res && res.ownProperties) { + desc[key].ownProperties = res.ownProperties; + } + } + } + }); + } + }); + + return ownProperties; + }, + /** * Find the XUL window that owns the content window. *