@samitouri / QOS-React-2 / commits / 365c031fd2

Workaround against display: inline bug in Safari (#32822)

Safari has a bug where if you put a block element inside an inline element and the inline element has a `view-transition-name` assigned it finds it as duplicate names. https://bugs.webkit.org/show_bug.cgi?id=290923 This adds a warning if we detect this scenario in dev mode. For the case where it renders into a single block, we can model this by making the parent either `block` or `inline-block` automatically to fix the issue. So we do that to automatically cover simple cases like `<a><div>...</div></a>`. This unfortunately causes layout/styling thrash so we might want to delete it once the bug has been fixed in enough Safari versions.

Sebastian Markbåge committed Apr 7, 2025 at 10:08 UTC 365c031fd2354e94248ed9390f13fe2975b994f6
3 files changed +122 -6
fixtures/view-transition/src/components/Page.js
+3 -1
@@ -113,7 +113,9 @@ export default function Page({url, navigate}) {
113
114 const exclamation = (
115 <ViewTransition name="exclamation" onShare={onTransition}>
116 - <span>!</span>
116 + <span>
117 + <div>!</div>
118 + </span>
119 </ViewTransition>
120 );
121 return (
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+116 -2
@@ -137,6 +137,11 @@ export type Props = {
137 'view-transition-name'?: string,
138 viewTransitionClass?: string,
139 'view-transition-class'?: string,
140 + margin?: string,
141 + marginTop?: string,
142 + 'margin-top'?: string,
143 + marginBottom?: string,
144 + 'margin-bottom'?: string,
145 ...
146 },
147 bottom?: null | number,
@@ -1161,6 +1166,59 @@ export function unhideTextInstance(
1166 textInstance.nodeValue = text;
1167 }
1168
1169 +function warnForBlockInsideInline(instance: HTMLElement) {
1170 + if (__DEV__) {
1171 + let nextNode = instance.firstChild;
1172 + outer: while (nextNode != null) {
1173 + let node: Node = nextNode;
1174 + if (
1175 + node.nodeType === ELEMENT_NODE &&
1176 + getComputedStyle((node: any)).display === 'block'
1177 + ) {
1178 + console.error(
1179 + "You're about to start a <ViewTransition> around a display: inline " +
1180 + 'element <%s>, which itself has a display: block element <%s> inside it. ' +
1181 + 'This might trigger a bug in Safari which causes the View Transition to ' +
1182 + 'be skipped with a duplicate name error.\n' +
1183 + 'https://bugs.webkit.org/show_bug.cgi?id=290923',
1184 + instance.tagName.toLocaleLowerCase(),
1185 + (node: any).tagName.toLocaleLowerCase(),
1186 + );
1187 + break;
1188 + }
1189 + if (node.firstChild != null) {
1190 + nextNode = node.firstChild;
1191 + continue;
1192 + }
1193 + if (node === instance) {
1194 + break;
1195 + }
1196 + while (node.nextSibling == null) {
1197 + if (node.parentNode == null || node.parentNode === instance) {
1198 + break;
1199 + }
1200 + node = node.parentNode;
1201 + }
1202 + nextNode = node.nextSibling;
1203 + }
1204 + }
1205 +}
1206 +
1207 +function countClientRects(rects: Array<ClientRect>): number {
1208 + if (rects.length === 1) {
1209 + return 1;
1210 + }
1211 + // Count non-zero rects.
1212 + let count = 0;
1213 + for (let i = 0; i < rects.length; i++) {
1214 + const rect = rects[i];
1215 + if (rect.width > 0 && rect.height > 0) {
1216 + count++;
1217 + }
1218 + }
1219 + return count;
1220 +}
1221 +
1222 export function applyViewTransitionName(
1223 instance: Instance,
1224 name: string,
@@ -1173,6 +1231,34 @@ export function applyViewTransitionName(
1231 // $FlowFixMe[prop-missing]
1232 instance.style.viewTransitionClass = className;
1233 }
1234 + const computedStyle = getComputedStyle(instance);
1235 + if (computedStyle.display === 'inline') {
1236 + // WebKit has a bug where assigning a name to display: inline elements errors
1237 + // if they have display: block children. We try to work around this bug in the
1238 + // simple case by converting it automatically to display: inline-block.
1239 + // https://bugs.webkit.org/show_bug.cgi?id=290923
1240 + const rects = instance.getClientRects();
1241 + if (countClientRects(rects) === 1) {
1242 + // If the instance has a single client rect, that means that it can be
1243 + // expressed as a display: inline-block or block.
1244 + // This will cause layout thrash but we live with it since inline view transitions
1245 + // are unusual.
1246 + const style = instance.style;
1247 + // If there's literally only one rect, then it's likely on a single line like an
1248 + // inline-block. If it's multiple rects but all but one of them are empty it's
1249 + // likely because it's a single block that caused a line break.
1250 + style.display = rects.length === 1 ? 'inline-block' : 'block';
1251 + // Margin doesn't apply to inline so should be zero. However, padding top/bottom
1252 + // applies to inline-block positioning which we can offset by setting the margin
1253 + // to the negative padding to get it back into original position.
1254 + style.marginTop = '-' + computedStyle.paddingTop;
1255 + style.marginBottom = '-' + computedStyle.paddingBottom;
1256 + } else {
1257 + // This case cannot be easily fixed if it has blocks but it's also fine if
1258 + // it doesn't have blocks. So we only warn in DEV about this being an issue.
1259 + warnForBlockInsideInline(instance);
1260 + }
1261 + }
1262 }
1263
1264 export function restoreViewTransitionName(
@@ -1180,6 +1266,7 @@ export function restoreViewTransitionName(
1266 props: Props,
1267 ): void {
1268 instance = ((instance: any): HTMLElement);
1269 + const style = instance.style;
1270 const styleProp = props[STYLE];
1271 const viewTransitionName =
1272 styleProp != null
@@ -1190,7 +1277,7 @@ export function restoreViewTransitionName(
1277 : null
1278 : null;
1279 // $FlowFixMe[prop-missing]
1193 - instance.style.viewTransitionName =
1280 + style.viewTransitionName =
1281 viewTransitionName == null || typeof viewTransitionName === 'boolean'
1282 ? ''
1283 : // The value would've errored already if it wasn't safe.
@@ -1205,12 +1292,39 @@ export function restoreViewTransitionName(
1292 : null
1293 : null;
1294 // $FlowFixMe[prop-missing]
1208 - instance.style.viewTransitionClass =
1295 + style.viewTransitionClass =
1296 viewTransitionClass == null || typeof viewTransitionClass === 'boolean'
1297 ? ''
1298 : // The value would've errored already if it wasn't safe.
1299 // eslint-disable-next-line react-internal/safe-string-coercion
1300 ('' + viewTransitionClass).trim();
1301 + if (style.display === 'inline-block') {
1302 + // We might have overridden the style. Reset it to what it should be.
1303 + if (styleProp == null) {
1304 + style.display = style.margin = '';
1305 + } else {
1306 + const display = styleProp.display;
1307 + style.display =
1308 + display == null || typeof display === 'boolean' ? '' : display;
1309 + const margin = styleProp.margin;
1310 + if (margin != null) {
1311 + style.margin = margin;
1312 + } else {
1313 + const marginTop = styleProp.hasOwnProperty('marginTop')
1314 + ? styleProp.marginTop
1315 + : styleProp['margin-top'];
1316 + style.marginTop =
1317 + marginTop == null || typeof marginTop === 'boolean' ? '' : marginTop;
1318 + const marginBottom = styleProp.hasOwnProperty('marginBottom')
1319 + ? styleProp.marginBottom
1320 + : styleProp['margin-bottom'];
1321 + style.marginBottom =
1322 + marginBottom == null || typeof marginBottom === 'boolean'
1323 + ? ''
1324 + : marginBottom;
1325 + }
1326 + }
1327 + }
1328 }
1329
1330 export function cancelViewTransitionName(
packages/react-reconciler/src/ReactFiberApplyGesture.js
+3 -3
@@ -834,18 +834,18 @@ function insertDestinationClonesOfFiber(
834 }
835
836 if (visitPhase === CLONE_EXIT || visitPhase === CLONE_UNHIDE) {
837 + appendChild(hostParentClone, clone);
838 + unhideInstance(clone, finishedWork.memoizedProps);
839 recursivelyInsertClones(
840 finishedWork,
841 clone,
842 null,
843 CLONE_APPEARING_PAIR,
844 );
843 - appendChild(hostParentClone, clone);
844 - unhideInstance(clone, finishedWork.memoizedProps);
845 trackHostMutation();
846 } else {
847 - recursivelyInsertClones(finishedWork, clone, null, visitPhase);
847 appendChild(hostParentClone, clone);
848 + recursivelyInsertClones(finishedWork, clone, null, visitPhase);
849 }
850 if (parentViewTransition !== null) {
851 if (parentViewTransition.clones === null) {