Closed Bug 1700565 Opened 7 months ago Closed 6 months ago

Facebook Pages - likes and follows detail section pushed on 2 rows with default scaling

Categories

(Core :: Layout: Text and Fonts, defect)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox86 --- wontfix
firefox87 --- wontfix
firefox88 --- wontfix
firefox89 --- wontfix
firefox90 --- fixed

People

(Reporter: cfogel, Assigned: TYLin)

References

(Regression, )

Details

(Keywords: regression)

Attachments

(6 files, 1 obsolete file)

Attached image 111d.png

Affected versions

  • 88.0b1,89.0a1 (2021-03-23);

Affected platforms

  • Windows 10

Steps to reproduce

  • DEFAULT zoom levels are set up;
  1. Launch Firefox, log in with a valid account;
  2. Access the Your Pages section;
  • can create a new page and add 1-2 likes to it;
  1. Observe the PAGES you manage section;

Expected result

  • elements are properly displayed;

Actual result

  • the value+likes section has a forced overflow even though there is enough space available to be displayed;

Regression range

Additional notes

  • set S4 as it might be a flaw in interaction with Facebook;
  • can be better observed when changing the zoom levels, as this gets triggered on unexpected values;
  • attached screenshot with the issue to best illustrate it.
Has Regression Range: --- → yes
Has STR: --- → yes

Updating based on regressor.

Component: Layout: Scrolling and Overflow → Layout: Flexbox
Regressed by: 1316534

Bug 1316534 fixed a longstanding webcompat issues in Firefox, so maybe it's best to reach out to Facebook team to see if they add workaround for old Firefox version.

In my experience, Facebook usually has complex layout, it would be very helpful to have a minimal testcase in order to be something actionable for our layout team.

Keywords: testcase-wanted

I can reproduce on Windows 10, though I can't reproduce on Linux or MacOS.

Here's a reduced testcase, from the Testcase Reducer add-on (again, this only triggers the problem on Win10):
https://jsfiddle.net/y2f5Lthx

The issue seems to be with the "." separator dots -- the layout devtools show that those only have a content size of 3.25px, but they end up with a minimum size of 4.12px for some reason, which then forces them to take that higher amount of width during flex layout. This leaves less width available for the other flex items, and so the "1 like" / "1 follow" tags end up with not-quite-enough-space to fit on one line.

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

The issue seems to be with the "." separator dots -- the layout devtools show that those only have a content size of 3.25px, but they end up with a minimum size of 4.12px for some reason, which then forces them to take that higher amount of width during flex layout. This leaves less width available for the other flex items, and so the "1 like" / "1 follow" tags end up with not-quite-enough-space to fit on one line.

Here's a screenshot (taken from the attached testcase, on Windows 10) showing this issue, as surfaced by our layout devtools pane.

So this seems to be a discrepancy between:

  • the way we do content measurement of flex items when determining the flex container's intrinsic width
  • the way we do content measurement of flex items when determining their automatic minimum size

And Segoe UI happens to be hitting some edge case where we come up with a different calculation for some reason (specifically when there is a particular bit of punctuation, surrounded by spaces).

I can reproduce this with the attached testcase on Linux, if I install the segoeui.ttf font (taken from Win10 on the same machine).

(I also had to reset layout.css.font-visibility.level to its default value, in order for this newly-installed font to be discoverable, because I had previously set that pref to 1 at some point in the past to dogfood a font-visibility experiment that jfkthame was working on.)

TYLin: now that we have a testcase, maybe you could take a look here? (Let me know if you don't have Windows machine to test on and/or to locally copy its segoeui.ttf font file from.)

I think this is an unintended behavior-change from Bug 1316534. That bug was focused on fixing something for flex items that have non-default flex-basis values; and here, the flex items have default styling, so I wouldn't have expected this testcase to change its behavior.

(Note: I verified that the reduced testcase does have the same regression range as Facebook does -- the range indicated in comment 0 here. In "good" builds, our flexbox layout devtools show that the content size and final size are both 3.47px; in "bad" builds, the automatic min size is larger and takes over.)

Flags: needinfo?(aethanyc)

Re comment 7:

And Segoe UI happens to be hitting some edge case where we come up with a different calculation for some reason (specifically when there is a particular bit of punctuation, surrounded by spaces).

Yes, I can see the discrepancy without using flexbox. Please see the attached testcase, case 2.

It's a <div> containing two characters, Middle Dot and a space. With Segoe UI font and word-break:break-word, it seems the trailing space can contribute to the block container's min-content width, so it's a bit wider than others.

Google Chrome gives all the cases the same width. We also do with font-family: serif or without word-break: break-word.

Jonathan, do you have any ideas?

Flags: needinfo?(aethanyc) → needinfo?(jfkthame)

Weird! Experimenting with testcase 1, it seems to occur when the punctuation character's width is narrower than the width of the space glyph. So in Segoe UI, for example, it reproduces with "!" but not with "?". (It also reproduces with "i" or "l"; it's not specific to punctuation.)

Some other fonts such as Tahoma can also reproduce (e.g. with "." or ",", but not with the slightly wider ":").

OK, I guess what's happening is that in the computation of min-content width, word-break: break-word (which is deprecated, fwiw, but overflow-wrap: anywhere results in the same effect) introduces a potential line break after the punctuation character and before the trailing space, which thus goes onto a second line by itself; and its width then contributes to the min-content measurement, ignoring the fact that it would actually hang. (But when the content is actually laid out, the space stays on the first line, mostly hanging beyond the box.)

This all seems a bit confused/broken. It does make me wonder, though: if an element contains only a hang-able space character, what should its min-content width be? Zero, because if the element occurs at line-end, it hangs and effectively occupies no space in its parent? Or the width of the space, because if the element isn't at line-end, it does need that much space to display its content?

Flags: needinfo?(jfkthame)

So this seems reminiscent of bug 1665657; perhaps simply a case that wasn't handled properly there?

See Also: → 1665657

Thanks for the info!

Jonathan, do you plan to fix this yourself? If not, I'm happy to dig the code that bug 1665657 touched and learn a bit more myself (hoping enough to fixing this bug).

Flags: needinfo?(jfkthame)

I'm not currently working on it, so if you have time to take a look that's fine.

Flags: needinfo?(jfkthame)
Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a4acaff2d290
Disallow spaces to contribute to minimum advance width when line breaking is everywhere. r=jfkthame
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28592 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Upstream PR merged by moz-wptsync-bot
Flags: in-testsuite+

The patch landed in nightly and beta is affected.
:TYLin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(aethanyc)

It's ok to let my patch ride the train because this bug actually exists for a while and is uncovered by bug 1316534.

Flags: needinfo?(aethanyc)
Keywords: testcase-wanted
Component: Layout: Flexbox → Layout: Text and Fonts
You need to log in before you can comment on or make changes to this bug.