Skip to content

Commit 10ae7db

Browse files
stwrtstwrt
committed
Web Inspector: Add Order Number to display: grid and grid-lanes
https://bugs.webkit.org/show_bug.cgi?id=304287 rdar://problem/166648769 Reviewed by BJ Burg and Devin Rousso. Add an "Order Numbers" option to CSS Grid overlays in Web Inspector, matching the existing functionality for Flexbox overlays. The implementation supports both regular CSS Grid layouts and CSS Masonry layouts. For regular grids, item bounds are computed from grid area lines. For masonry layouts, the item's frame rect is used directly since grid areas may not have definite positions in the masonry axis. * LayoutTests/inspector/dom/showFlexOverlay-expected.txt: * LayoutTests/inspector/dom/showFlexOverlay.html: * LayoutTests/inspector/dom/showGridOverlay-expected.txt: * LayoutTests/inspector/dom/showGridOverlay.html: * Source/JavaScriptCore/inspector/protocol/DOM.json: * Source/WebCore/en.lproj/Localizable.strings: * Source/WebCore/inspector/InspectorOverlay.cpp: (WebCore::InspectorOverlay::clearGridOverlayForNode): (WebCore::InspectorOverlay::clearFlexOverlayForNode): (WebCore::InspectorOverlay::buildGridOverlay): (WebCore::InspectorOverlay::buildFlexOverlay): * Source/WebCore/inspector/InspectorOverlay.h: * Source/WebCore/inspector/agents/InspectorDOMAgent.cpp: (WebCore::InspectorDOMAgent::gridOverlayConfigFromInspectorObject): * Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js: * Source/WebInspectorUI/UserInterface/Base/Setting.js: * Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js: (WI.DOMManager.buildHighlightConfigs): * Source/WebInspectorUI/UserInterface/Models/DOMNode.js: (WI.DOMNode.prototype.set layoutFlags): (WI.DOMNode.prototype.showLayoutOverlay): (WI.DOMNode.prototype.hideLayoutOverlay): * Source/WebInspectorUI/UserInterface/Views/LayoutDetailsSidebarPanel.js: (WI.LayoutDetailsSidebarPanel.prototype.initialLayout): Canonical link: https://commits.webkit.org/304645@main
1 parent 9a526e4 commit 10ae7db

File tree

14 files changed

+133
-32
lines changed

14 files changed

+133
-32
lines changed

‎LayoutTests/inspector/dom/showFlexOverlay-expected.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,10 @@ PASS: Should have 0 flex overlays displayed.
3434
Requesting to hide all flex overlays again, expecting none to be cleared. Hiding all flex overlays is idempotent and should not throw an error.
3535
PASS: Should have 0 flex overlays displayed.
3636

37-
-- Running test case: DOM.showFlexOverlay.HideBeforeShowShouldError
37+
-- Running test case: DOM.showFlexOverlay.HideBeforeShowShouldSucceed
38+
PASS: Should have 0 flex overlays displayed.
39+
Requesting to hide flex overlay for .flex-container (should silently succeed)
3840
PASS: Should have 0 flex overlays displayed.
39-
Requesting to hide flex overlay for .flex-container
40-
PASS: Should produce an exception.
41-
Error: No flex overlay exists for the node, so cannot clear.
4241
Requesting to hide all flex overlays. Hiding all flex overlays is idempotent and should not throw an error.
4342
PASS: Should have 0 flex overlays displayed.
4443

‎LayoutTests/inspector/dom/showFlexOverlay.html‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,16 @@
120120
});
121121

122122
suite.addTestCase({
123-
name: "DOM.showFlexOverlay.HideBeforeShowShouldError",
124-
description: "Return an error when requesting to hide a flex overlay when none is active for the node.",
123+
name: "DOM.showFlexOverlay.HideBeforeShowShouldSucceed",
124+
description: "No error when requesting to hide a flex overlay when none is active for the node.",
125125
async test() {
126126
let container = await getFlexContainerNode();
127127

128128
await checkFlexOverlayCount(0);
129129

130-
InspectorTest.log("Requesting to hide flex overlay for .flex-container");
131-
await InspectorTest.expectException(async () => {
132-
await DOMAgent.hideFlexOverlay(container.id);
133-
});
130+
InspectorTest.log("Requesting to hide flex overlay for .flex-container (should silently succeed)");
131+
await DOMAgent.hideFlexOverlay(container.id);
132+
await checkFlexOverlayCount(0);
134133

135134
InspectorTest.log("Requesting to hide all flex overlays. Hiding all flex overlays is idempotent and should not throw an error.");
136135
await DOMAgent.hideFlexOverlay();

‎LayoutTests/inspector/dom/showGridOverlay-expected.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,10 @@ PASS: Should have 0 grids displayed.
4646
Requesting to hide all grid overlays again, expecting none to be cleared. Hiding all grids is idempotent and should not throw an error.
4747
PASS: Should have 0 grids displayed.
4848

49-
-- Running test case: DOM.showGridOverlay.HideBeforeShowShouldError
49+
-- Running test case: DOM.showGridOverlay.HideBeforeShowShouldSucceed
50+
PASS: Should have 0 grids displayed.
51+
Requesting to hide grid overlay for .grid-container (should silently succeed)
5052
PASS: Should have 0 grids displayed.
51-
Requesting to hide grid overlay for .grid-container
52-
PASS: Should produce an exception.
53-
Error: No grid overlay exists for the node, so cannot clear.
5453
Requesting to hide all grid overlays. Hiding all grids is idempotent and should not throw an error.
5554
PASS: Should have 0 grids displayed.
5655

‎LayoutTests/inspector/dom/showGridOverlay.html‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,16 @@
124124
});
125125

126126
suite.addTestCase({
127-
name: "DOM.showGridOverlay.HideBeforeShowShouldError",
128-
description: "Return an error when requesting to hide a grid overlay when none is active for the node.",
127+
name: "DOM.showGridOverlay.HideBeforeShowShouldSucceed",
128+
description: "No error when requesting to hide a grid overlay when none is active for the node.",
129129
async test() {
130130
let container = await getGridContainerNode();
131131

132132
await checkGridOverlayCount(0);
133133

134-
InspectorTest.log("Requesting to hide grid overlay for .grid-container");
135-
await InspectorTest.expectException(async () => {
136-
await DOMAgent.hideGridOverlay(container.id);
137-
});
134+
InspectorTest.log("Requesting to hide grid overlay for .grid-container (should silently succeed)");
135+
await DOMAgent.hideGridOverlay(container.id);
136+
await checkGridOverlayCount(0);
138137

139138
InspectorTest.log("Requesting to hide all grid overlays. Hiding all grids is idempotent and should not throw an error.");
140139
await DOMAgent.hideGridOverlay();

‎Source/JavaScriptCore/inspector/protocol/DOM.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@
180180
{ "name": "showLineNumbers", "type": "boolean", "optional": true, "description": "Show labels for grid line numbers. If not specified, the default value is false." },
181181
{ "name": "showExtendedGridLines", "type": "boolean", "optional": true, "description": "Show grid lines that extend beyond the bounds of the grid. If not specified, the default value is false." },
182182
{ "name": "showTrackSizes", "type": "boolean", "optional": true, "description": "Show grid track size information. If not specified, the default value is false." },
183-
{ "name": "showAreaNames", "type": "boolean", "optional": true, "description": "Show labels for grid area names. If not specified, the default value is false." }
183+
{ "name": "showAreaNames", "type": "boolean", "optional": true, "description": "Show labels for grid area names. If not specified, the default value is false." },
184+
{ "name": "showOrderNumbers", "type": "boolean", "optional": true, "description": "Show labels for grid item order. If not specified, the default value is false." }
184185
]
185186
},
186187
{

‎Source/WebCore/en.lproj/Localizable.strings‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,9 @@
805805
/* Undo action name */
806806
"Italics (Undo action name)" = "Italics";
807807

808+
/* Inspector Grid/Flex Item DOM order label */
809+
"Item #%lu" = "Item #%lu";
810+
808811
/* WebKitErrorJavaUnavailable description */
809812
"Java is unavailable" = "Java is unavailable";
810813

@@ -1156,6 +1159,9 @@
11561159
/* Codec Strings */
11571160
"Opus Codec String" = "Opus";
11581161

1162+
/* Inspector Grid/Flex Item CSS order property label */
1163+
"order: %d" = "order: %d";
1164+
11591165
/* Label for the order with Apple Pay button. */
11601166
"Order with Apple Pay" = "Order with Apple Pay";
11611167

‎Source/WebCore/inspector/InspectorOverlay.cpp‎

Lines changed: 92 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -742,10 +742,9 @@ ErrorStringOr<void> InspectorOverlay::setGridOverlayForNode(Node& node, const In
742742

743743
ErrorStringOr<void> InspectorOverlay::clearGridOverlayForNode(Node& node)
744744
{
745-
if (!removeGridOverlayForNode(node))
746-
return makeUnexpected("No grid overlay exists for the node, so cannot clear."_s);
747-
748-
update();
745+
// Silently succeed if no overlay exists - the frontend may not know the exact overlay type.
746+
if (removeGridOverlayForNode(node))
747+
update();
749748

750749
return { };
751750
}
@@ -781,10 +780,9 @@ ErrorStringOr<void> InspectorOverlay::setFlexOverlayForNode(Node& node, const In
781780

782781
ErrorStringOr<void> InspectorOverlay::clearFlexOverlayForNode(Node& node)
783782
{
784-
if (!removeFlexOverlayForNode(node))
785-
return makeUnexpected("No flex overlay exists for the node, so cannot clear."_s);
786-
787-
update();
783+
// Silently succeed if no overlay exists - the frontend may not know the exact overlay type.
784+
if (removeFlexOverlayForNode(node))
785+
update();
788786

789787
return { };
790788
}
@@ -1866,6 +1864,90 @@ std::optional<InspectorOverlay::Highlight::GridHighlightOverlay> InspectorOverla
18661864
}
18671865
}
18681866

1867+
if (gridOverlay.config.showOrderNumbers) {
1868+
Vector<RenderBox*> gridItemsInGridOrder;
1869+
Vector<RenderBox*> gridItemsInDOMOrder;
1870+
bool hasCustomOrder = false;
1871+
1872+
auto& orderIterator = renderGrid.currentGrid().orderIterator();
1873+
for (auto* gridItem = orderIterator.first(); gridItem; gridItem = orderIterator.next()) {
1874+
if (orderIterator.shouldSkipChild(*gridItem))
1875+
continue;
1876+
gridItemsInGridOrder.append(gridItem);
1877+
}
1878+
1879+
for (auto* child = node->firstChild(); child; child = child->nextSibling()) {
1880+
if (auto* renderer = dynamicDowncast<RenderBox>(child->renderer())) {
1881+
if (!gridItemsInGridOrder.contains(renderer))
1882+
continue;
1883+
1884+
gridItemsInDOMOrder.append(renderer);
1885+
1886+
if (!renderer->style().order().isZero())
1887+
hasCustomOrder = true;
1888+
}
1889+
}
1890+
1891+
for (auto* gridItem : gridItemsInGridOrder) {
1892+
FloatQuad itemBounds;
1893+
1894+
if (renderGrid.isMasonry()) {
1895+
// For masonry layouts, use absoluteBoundingBoxRect to get the visual position
1896+
// accounting for all scroll offsets and transforms including zoom.
1897+
auto absoluteRect = FloatRect { gridItem->absoluteBoundingBoxRect(true) };
1898+
auto margins = gridItem->marginBox();
1899+
absoluteRect.expand(FloatBoxExtent { margins.top(), margins.right(), margins.bottom(), margins.left() });
1900+
itemBounds = FloatQuad {
1901+
localPointToRootPoint(containingView, absoluteRect.minXMinYCorner()),
1902+
localPointToRootPoint(containingView, absoluteRect.maxXMinYCorner()),
1903+
localPointToRootPoint(containingView, absoluteRect.maxXMaxYCorner()),
1904+
localPointToRootPoint(containingView, absoluteRect.minXMaxYCorner())
1905+
};
1906+
} else {
1907+
// For regular grid layouts, compute bounds from the grid area.
1908+
auto gridArea = renderGrid.currentGrid().gridItemArea(*gridItem);
1909+
if (!gridArea.rows.isTranslatedDefinite() || !gridArea.columns.isTranslatedDefinite())
1910+
continue;
1911+
1912+
auto columnStartIndex = gridArea.columns.startLine();
1913+
auto columnEndIndex = gridArea.columns.endLine() - 1;
1914+
auto rowStartIndex = gridArea.rows.startLine();
1915+
auto rowEndIndex = gridArea.rows.endLine() - 1;
1916+
1917+
if (columnStartIndex >= columnPositions.size() || columnEndIndex >= columnWidths.size())
1918+
continue;
1919+
if (rowStartIndex >= rowPositions.size() || rowEndIndex >= rowHeights.size())
1920+
continue;
1921+
1922+
auto columnStartLine = columnLineAt(columnPositions[columnStartIndex]);
1923+
auto columnEndLine = columnLineAt(columnPositions[columnEndIndex] + columnWidths[columnEndIndex]);
1924+
auto rowStartLine = rowLineAt(rowPositions[rowStartIndex]);
1925+
auto rowEndLine = rowLineAt(rowPositions[rowEndIndex] + rowHeights[rowEndIndex]);
1926+
1927+
std::optional<FloatPoint> topLeft = columnStartLine.intersectionWith(rowStartLine);
1928+
std::optional<FloatPoint> topRight = columnEndLine.intersectionWith(rowStartLine);
1929+
std::optional<FloatPoint> bottomRight = columnEndLine.intersectionWith(rowEndLine);
1930+
std::optional<FloatPoint> bottomLeft = columnStartLine.intersectionWith(rowEndLine);
1931+
1932+
if (!topLeft || !topRight || !bottomRight || !bottomLeft)
1933+
continue;
1934+
1935+
itemBounds = { *topLeft, *topRight, *bottomRight, *bottomLeft };
1936+
}
1937+
1938+
StringBuilder orderNumbers;
1939+
1940+
if (auto index = gridItemsInDOMOrder.find(gridItem); index != notFound)
1941+
orderNumbers.append(WEB_UI_FORMAT_STRING("Item #%lu", "Inspector Grid Item DOM order label", static_cast<unsigned long>(index + 1)));
1942+
1943+
if (auto order = gridItem->style().order(); !order.isZero() || hasCustomOrder)
1944+
orderNumbers.append(orderNumbers.isEmpty() ? ""_s : "\n"_s, WEB_UI_FORMAT_STRING("order: %d", "Inspector Grid Item CSS order property label", order.value));
1945+
1946+
if (!orderNumbers.isEmpty())
1947+
gridHighlightOverlay.labels.append({ orderNumbers.toString(), itemBounds.center(), Color::white.colorWithAlphaByte(230), { InspectorOverlayLabel::Arrow::Direction::None, InspectorOverlayLabel::Arrow::Alignment::None } });
1948+
}
1949+
}
1950+
18691951
return { gridHighlightOverlay };
18701952
}
18711953

@@ -2070,10 +2152,10 @@ std::optional<InspectorOverlay::Highlight::FlexHighlightOverlay> InspectorOverla
20702152
StringBuilder orderNumbers;
20712153

20722154
if (auto index = renderChildrenInDOMOrder.find(renderChild); index != notFound)
2073-
orderNumbers.append("Item #"_s, index + 1);
2155+
orderNumbers.append(WEB_UI_FORMAT_STRING("Item #%lu", "Inspector Flex Item DOM order label", static_cast<unsigned long>(index + 1)));
20742156

20752157
if (auto order = renderChild->style().order(); !order.isZero() || hasCustomOrder)
2076-
orderNumbers.append(orderNumbers.isEmpty() ? ""_s : "\n"_s, "order: "_s, order.value);
2158+
orderNumbers.append(orderNumbers.isEmpty() ? ""_s : "\n"_s, WEB_UI_FORMAT_STRING("order: %d", "Inspector Flex Item CSS order property label", order.value));
20772159

20782160
if (!orderNumbers.isEmpty())
20792161
flexHighlightOverlay.labels.append({ orderNumbers.toString(), itemBounds.center(), Color::white.colorWithAlphaByte(230), { InspectorOverlayLabel::Arrow::Direction::None, InspectorOverlayLabel::Arrow::Alignment::None } });

‎Source/WebCore/inspector/InspectorOverlay.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ class InspectorOverlay : public CanMakeWeakPtr<InspectorOverlay> {
163163
bool showExtendedGridLines;
164164
bool showTrackSizes;
165165
bool showAreaNames;
166+
bool showOrderNumbers;
166167
};
167168

168169
WeakPtr<Node, WeakPtrImplWithEventTargetData> gridNode;

‎Source/WebCore/inspector/agents/InspectorDOMAgent.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,7 @@ std::optional<InspectorOverlay::Grid::Config> InspectorDOMAgent::gridOverlayConf
14091409
gridOverlayConfig.showExtendedGridLines = gridOverlayInspectorObject->getBoolean("showExtendedGridLines"_s).value_or(false);
14101410
gridOverlayConfig.showTrackSizes = gridOverlayInspectorObject->getBoolean("showTrackSizes"_s).value_or(false);
14111411
gridOverlayConfig.showAreaNames = gridOverlayInspectorObject->getBoolean("showAreaNames"_s).value_or(false);
1412+
gridOverlayConfig.showOrderNumbers = gridOverlayInspectorObject->getBoolean("showOrderNumbers"_s).value_or(false);
14121413
return gridOverlayConfig;
14131414
}
14141415

‎Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,7 @@ localizedStrings["Option-click to show source"] = "Option-click to show source";
12321232
/* Tooltip with instructions on how to show all hidden CSS variables */
12331233
localizedStrings["Option-click to show unused CSS variables from all rules @ Styles Sidebar Panel Tooltip"] = "Option-click to show unused CSS variables from all rules";
12341234
localizedStrings["Options"] = "Options";
1235-
/* Label for option to toggle the order numbers setting for CSS flex overlays */
1235+
/* Label for option to toggle the order numbers setting for CSS grid and flex overlays */
12361236
localizedStrings["Order Numbers @ Layout Panel Overlay Options"] = "Order Numbers";
12371237
/* Property value for `font-variant-numeric: ordinal`. */
12381238
localizedStrings["Ordinal Letter Forms @ Font Details Sidebar Property Value"] = "Ordinal Letter Forms";

0 commit comments

Comments
 (0)