50959 - [MARGIN-C]block vertical margins need rewrite

Status: NEW

(Core :: Layout: Block and Inline, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm giving up on my attempts to fix most of the bugs in vertical margins on
blocks.  My changes are doing things that I didn't expect them to do, and I just
don't have the energy to look at this code anymore to figure out why not.  The
original changes were better in terms of correctness but crashed in some cases
(e.g., tinderbox popups).  The #if 0 code shows my original changes.

I will attach the changes that I am currently removing from my tree.  I will
attempt to pull the one or two easy fixes out of these changes and try and get
them checked in.  However, the margin code has serious problems and I don't
understand the block code well enough to fix them.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: work in progress that I am giving up on

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The changes here basically fix bug 50142 and bug 44242.  There are also other
related MARGIN-C bugs, some of which are compat issues that are less important
once we have released and set a precedent.  Some of those compat issues may be
fixed by these changes (or an improvement of these changes) as well.

Comment 3

[Ben Bucksch (:BenB)]

dbaron, which bugs did you try to fix but failed (i.e. which other bugs would
this fix apart from bugs 44242,50142)?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Those are the bugs I succeeded in fixing with these changes.  I was trying 
to fix many of the other bugs marked [MARGIN-C], e.g., bug 18206 (I may 
have had that fixed too), bug 29413, and bug 34404.  I hadn't yet thought of 
a good way to fix the compatibility [MARGIN-C] bugs.

Comment 5

[Jason Eager]

Is anyone else here willing to pick these changes up and run with them? I'm
currently trying to get involved with fixing layout bugs, but I've got a bunch
of learning to do first before I can really help fix bugs of this scope.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Taking back.  Part of what the original rewrite did, plus other things, is being
done for bug 86947.

Comment 7

[Markus Hübner]

Has there been any progress in the meantime?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So roc rewrote them again more recently, regressing performance significantly, and I think we need another rewrite, so this is still valid.

Comment 9

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

The severity field for this bug is relatively low, S3. However, the bug has 10 votes.
:jfkthame, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 10

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.