Closed Bug 1406619 Opened 2 years ago Closed 2 months ago

Two-column layout balanced unevenly and nondeterministically since Firefox 56

Categories

(Core :: Layout, defect, P3)

56 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- wontfix
firefox57 + wontfix
firefox58 + wontfix

People

(Reporter: simmo.saan, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171003101020

Steps to reproduce:

Visited https://courses.cs.ut.ee/.


Actual results:

The two columns had uneven heights (most elements in first column, only a few in second), page is over million pixels long (see scroll bar) - screenshot attached.


Expected results:

Two columns with even heights, no empty excess space. Before upgrading to Firefox 56, it always used to display like that. Another computer with Firefox 55 still displays correctly so it's not a change on the site causing this. Chromium displays it properly too.
The layout issue exhibits some nondeterministic behavior:
* It doesn't happen on every visit.
* The column split happens after different element at different times.
* The empty spaces doesn't happen every time.
* Opening Inspector might cause the layout to change for no good reason.

Confirmed also on moznet#firefox:
< Timvde> sim642: cannot reproduce
[...]
< Timvde> sim642: Strange, loading a *second time*, I can reproduce it
< Timvde> But it looked fine the first time
* Timvde is confused
Can you use http://mozilla.github.io/mozregression/ to determine a regression range?
Flags: needinfo?(simmo.saan)
I did mozregression between release 55 and date 2017-10-07 (today, mozregression-gui doesn't have release 56) and ended up with this:
Bug 1308876 - Add crashtest that used to hang Firefox.  r=dholbert
MozReview-Commit-ID: 2ZCrhcV5i2G

It was a bit difficult to bisect because the issue doesn't always reveal itself immediately (some refreshing required to be more sure about "good") but looking at Bug 1308876 it looks like it could be relevant as it has to do with column reflow and break-inside: avoid, which the linked broken site also seems to try to use at least.
Flags: needinfo?(simmo.saan)
I'm attaching a saved copy of the problematic page linked and tested with in case the original URL gets modified to work around this issue, making it harder to confirm and test this. The saved copy does not exhibit all the problems of the original (possibly due to some lacking files somehow) but the uneven columns and their jumpy nondeterministic behavior is still observable.
[Tracking Requested - why for this release]:Layout regression.


I can reproduce the problem after reload(f5) the page on Windows10 Nightly58.0a1.


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e6e712904806da25a9c8f48ea4533abe7c6ea8f4&tochange=d6bf703c5deaf1e328babd03d5e68ff2a4ffe10e

Via local build,
Last good: 395b6c53e42b
First bad: 1e3130e96f03

Regressed by: Bug 1308876

@:dbaron
Your bunch of patch seems to cause the problem. Can you look at this?
Blocks: 1308876
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Flags: needinfo?(dbaron)
Keywords: regression
Product: Firefox → Core
Tracking as recent regression.
FYI -- I'm aware that a bunch of regressions from bug 1308876 turned up after it hit release (after none were reported while it was on nightly or beta).  See https://bugzilla.mozilla.org/show_bug.cgi?id=1308876#a30998038_3881 and below.  I'm going to try to look into them over the next week or two -- and hopefully there are fewer underlying problems than there are bug reports -- but these can be somewhat difficult bugs, so it might take a little time.
Priority: -- → P3
Summary: Two-column layout broken since Firefox 56 → Two-column layout balanced unevenly and nondeterministically since Firefox 56
Too late to fix in 57 but we could still potentially take patches for 58.
I'm not familiar with Mozilla's and Firefox's take on dealing with regressions but I am finding the lack of activity and seriousness with regard to a serious regression appalling. This regression has essentially broken CSS columns support in Firefox, or at least made it impossible for any web developer to rely on to work, as it not only breaks column balance, also introduces a million-pixel unused layout space, which isn't acceptable for any visitor on any site. I completely understand that a bug has made it into a release but what I don't understand is why a regression, found in timely manner to be eliminated from further releases, is just being left to be. If the root cause hasn't been found, there's always one solution: revert the regressing patch, which is especially obvious since it has been pinned down, and reapply a fixed patch in the future that deals with the multiple bugs and regressions it initially introduced and failed to address.

The choice is essentially between
1. optimized Firefox that fails to do what it has been able to for a long time;
2. unoptimized Firefox in a specific case that works as before and expected.
It is very odd to prefer broken Firefox to a not-so-broken one. The current situation is well-described as "it doesn't work but it's fast" (http://theprofoundprogrammer.com/post/28974600028/text-it-doesnt-work-but-its-fast), which contradicts any sensible approach to optimizing software, because anything can be optimized further by removing some existing functionality, but the real art is, of course, to do it without breaking anything that has always worked and should continue to work. Might just as well optimize column layout by using rand() directly at this point because the user won't notice the difference.

By not doing what is possible to avoid users from having a horrible experience on a site that uses CSS columns, which used to be just fine, the message being sent is that we prefer keeping patches (even if known to have caused regressions), incubating and slowly fixing them in production and not dealing with known regressions, to providing the users the best possible experience, which was better before.
Flags: needinfo?(dbaron)
No longer blocks: 1308876
Regressed by: 1308876

This was fixed in this range in December 2017. I don't see why, although bug 1424524 is possible.

Sorry for letting this sit around so long.

Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.