551699 - auto y offset for position:absolute in table is incorrect

Status: VERIFIED FIXED

(Core :: Layout: Positioned, defect, P2)

Description

[Mathias Tschopp]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100308 Minefield/3.7a3pre

In the test case linked, the text should appear overlayed on top of the image but in the latest 1.9.3 there's a regression and it appears at the bottom.

Reproducible: Always

Steps to Reproduce:
1.Load test case URL
2.Compare to rendering on 1.9.2

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 522632.  In particular, we now end up with the cellcontent as the hypothetical box container instead of ending up with its block as the hypothetical box container.

In particular, we used to stop our hypothetical box container search at the closest ancestor block or containing block, whichever we hit first.  In practice, for a placeholder, the only way to hit a block that wasn't a containing block was to hit an ib-split anon block, and we in fact have other blocks between those and placeholders at all times now, so that couldn't happen either.  Now that cellcontent blocks are not containing blocks, we walk past it and stop at the cell instead.  Then in CalculateHypotheticalBox we use the placeholder position instead of the line top, since the passed-in containing block is not a block.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I think using the cellcontent block as the hypothetical box container is probably the right thing to do.  Patch coming up.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Upon some more reflection, I decided to keep the containing block as the containing block but use its content insertion frame as needed for the hypothetical box calculation.

Comment 4

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

Comment on attachment 431914 [details] [diff] [review]
Fix

r=dbaron

Comment 7

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

Boris, could you double-check that this patch fixes the testcase in the duplicate?  I didn't check that.  (It sure matches the description of this bug.)

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> *** Bug 553314 has been marked as a duplicate of this bug. ***

Ah! I searched for bugs with the regression keyword only :-/

Bug 522632 is in the regression timeframe there ;-)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/00155ff66a5d

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 431914 [details] [diff] [review]
Fix

We need to get this layout correctness fix on the 1.9.2 branch too.  It's quite safe.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Boris, could you double-check that this patch fixes the testcase in the
> duplicate?

It does, yes.

Comment 13

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a4pre) Gecko/20100403 SeaMonkey/2.1a1pre] (nightly)

V.Fixed, per bug 553314 testcase and real-life uses.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 431914 [details] [diff] [review]
Fix

Clearing old approval requests now that 1.9.2.4 has shipped. If you believe this patch is still necessary on the 1.9.2 branch please re-request approval along with a risk/benefit analysis explaining why we need it.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 431914 [details] [diff] [review]
Fix

This is a reasonably serious web compat regression, with multiple duplicates.  The risk is, in my opinion fairly low, and the impact is that sites are broken right now.

Comment 16

[Boris Zbarsky [:bzbarsky]]

And at the risk of sounding repetitive, it's been close to 3 months since I made the approval request, and this is the first anyone's looked at it, apparently...  If we intend to take functionality regressions on branches seriously (and it's not clear to me that we do, so far), do I need to escalate them to blocker requests or something?

Comment 17

[Daniel Veditz [:dveditz]]

This bug is marked as a regression from bug 522632, and that bug was only checked into mozilla-central well after the 1.9.2 branch as far as we can tell. Does this really affect 1.9.2 and need to be checked in there? In the linked testcase "should be on top" is in fact on top when I try it.

> If we intend to take functionality regressions on branches seriously
> (and it's not clear to me that we do, so far), do I need to escalate
> them to blocker requests or something?

Yes, regressions from patches previously taken on the branch are blockers, please nominate them as such. Regressions between major releases are not automatically blockers (they obviously didn't block the first version) but if you think it's serious and was an oversight you can nominate those, too.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 431914 [details] [diff] [review]
Fix

> and that bug was only checked into mozilla-central well after the 1.9.2 branch

Ah, indeed.  For some reason I thought it had landed on the branch (possibly because it fixed various correctness issues and at one point I had considered backporting it).  You're right; this patch is not needed on branch.  Sorry for the noise.  :(