1936276 - beta.destinyitemmanager.com - Incorrect wrapping causing overlapping on items

Status: RESOLVED FIXED

(Core :: Layout: Grid, defect, P1)

Description

[Raul Bucata]

Environment:
Operating system: Windows 10
Firefox version: Firefox 134.0

Steps to reproduce:

1. Navigate to: https://jeffgca.github.io/dim-fx-bug/
2. Observe the layout of items

https://github.com/DestinyItemManager/DIM/issues/10820

Expected Behavior:
Items are correctly wrapped

Actual Behavior:
Items are overlapped

Notes:

• Reproduces regardless of the status of ETP
• Reproduces in firefox-nightly, and firefox-release
• Does not reproduce in chrome

Created from https://github.com/webcompat/web-bugs/issues/144955

Comment 1

[Raul Bucata]

The icons inside the items seem broken as well, as Chrome show empty rectangles. If a new, separate bug is to be submitted, please let me know

Comment 2

[Raul Bucata]

Attachment: ff vs chrome

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fc9552282ebd73c732e2e9ba46b9a84839ef02de&tochange=4f3dfc147a61ef539dee209d16ebf92bdfd3f656

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1930672

:TYLin, since you are the author of the regressor, bug 1930672, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 6

[Ben Hollis]

The empty icon boxes are just due to a missing font - the repro could be minimized further, but the important issue is the overlapping boxes containing the icons due to an incorrect width calculation.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (barely-reduced via testcase reducer addon)

Comment 8

[Daniel Holbert [:dholbert]]

possible site-side workaround:

If anyone here happens to be a developer on the site or be in touch with the developers, this seems to be an effective workaround:

.sub-bucket {
  min-width: 0;
}

The issue here is that Firefox is resolving too large of an automatic minimum width and refusing to shrink the "sub-bucket" grid (which itself is a flex item) because we're running into what-we-think-is-its-minimum-size.

If we allow it to shrink, though (e.g. via that min-width), then everything appears to work out just fine.

And from some brief testing, I don't think this css rule impacts the sizing in Chrome at all, which is good (in the sense of it being a harmless workaround).

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced)

Comment 10

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: regression that's made it to beta, causing overlapping content, that would ideally be good to avoid shipping.

TYLin: it looks like the regressor (bug 1930672) was pretty targeted -- perhaps we should request a backout from beta, to have more time to investigate/address this bug here before it hits release?

Comment 11

[Daniel Holbert [:dholbert]]

Important correction for tracking/prioritization/next-action purposes:
(In reply to Raul Bucata from comment #0)

Notes:

• Reproduces regardless of the status of ETP
• Reproduces in firefox-nightly, and firefox-release

This actually does not reproduce in firefox-release, at least not for me (not yet). Per regression window in comment 3, this regressed from a patch in bug 1930672 that is currently included in 134 (current beta) but was not included in 133 (current release).

(It looks like this matches the original reporter's experience, too -- in https://github.com/webcompat/web-bugs/issues/144955#issuecomment-2526270353 they show a screenshot of nightly 135 with overlap, followed by Chrome with no overlap and release 133 with no overlap.)

Comment 12

[Daniel Holbert [:dholbert]]

[Morphing into a regular platform bug, since this is a regression rather than a longstanding webcompat issue.]

Comment 13

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1936276 - Revert Bug 1930672 to fix the overlapping icons on beta.destinyitemmanager.com. r?#layout

This reverts commit https://hg.mozilla.org/mozilla-central/rev/4f3dfc147a61
after resolving the code conflict with Bug 1933101 Part 2
https://hg.mozilla.org/mozilla-central/rev/03da611d9c58

Comment 14

[jeff]

I’m the original reporter of the webcompat bug and that’s correct, I wasn’t able to reproduce in current release version. Not sure where that crept in - the upstream bug for the site was reported 12 days ago:

https://github.com/DestinyItemManager/DIM/issues/10820

( DIM is a website companion app for a major online game, for some context )

Comment 15

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1936276 - Add web-platform tests for grid container as flex item with auto repeat columns. r?dholbert

flex-item-grid-container-auto-repeat-001.html is adapted from Daniel Holbert's
reduced testcase in Bug 1936276 Comment 9.

flex-item-grid-container-auto-repeat-002.html is identical to 001.html except
that the width of .item-grid-container are set to a pixel value rather than
a percentage values. This test has failed before introducing the regression.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5e36f13e8cd
Revert Bug 1930672 to fix the overlapping icons on beta.destinyitemmanager.com. r=layout-reviewers,dholbert
https://hg.mozilla.org/integration/autoland/rev/bea77c09d334
Add web-platform tests for grid container as flex item with auto repeat columns. r=dholbert

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49660 for changes under testing/web-platform/tests

Comment 18

[Daniel Holbert [:dholbert]]

(ni=tylin as a reminder to request beta uplift when possible [after it lands on central], to avoid shipping this regression in 134. Thanks!)

Comment 19

[Daniel Holbert [:dholbert]]

(In reply to jeff from comment #14)

I’m the original reporter of the webcompat bug and that’s correct, I wasn’t able to reproduce in current release version. Not sure where that crept in - the upstream bug for the site was reported 12 days ago:

Thanks for the upstream bug reference!

We did manage to track down where the bug crept in, and probably it'll be fixed in tomorrow's Nightly build. (and hopefully fixed in Firefox Beta 134 within a week or maybe sooner, depending on beta update timelines etc)

Comment 20

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #18)

(ni=tylin as a reminder to request beta uplift when possible [after it lands on central], to avoid shipping this regression in 134. Thanks!)

Aha, I see we backed out on beta already here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1930672#c11

(The process was just slightly different for trunk because of the merge conflict, which is why we had the custom patch here.)

So we can consider Firefox 134beta fixed-by-backout, I think (just as 135 will be fixed-by-backout as soon as the patch merges from autoland to central).

Comment 21

[amarc]

https://hg.mozilla.org/mozilla-central/rev/d5e36f13e8cd
https://hg.mozilla.org/mozilla-central/rev/bea77c09d334

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot