Table Alignment. Worked just fine in Firefox2.0.14/15 but not Firefox 3

VERIFIED FIXED

Status

()

VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: loz, Assigned: bernd_mozilla)

Tracking

({regression, testcase})

Trunk
x86
Windows Vista
regression, testcase
Points:
---
Bug Flags:
wanted1.9.1 +
blocking1.9.0.2 -
wanted1.9.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

Hi

I have a site called http://www.videoguide-toprofits.com
as you can see, the text is moved over to the left of the
main content holder, it's a normal table that has been
centered with text in side the table. When I developed this
in Firefox 2.0.14/15 all appeared to work just fine, now,
since you've updated to firefox 3 and over 8 million people
have upgraded, it would appear to me that a lot of sites
out there do not appear what they looked like in Firefox2.

This is also the same for IE8 as well, even though they are
in beta, firefox 3 is a published release that still seems
buggy.

Would you know when this will be corrected. I am a website
designer and I can no longer design sites in Firefox 3, and
it wont be long before the majority will upgrade to FF3.

Thanks


Looking forward to your reply.

Yours VERY concerned.

Loz

Reproducible: Always

Steps to Reproduce:
1. No need to reproduce error, worked find in firefox 2 and IE7 (but not in IE8)
2.
3.
Actual Results:  
The table isn't totally centered like it was in firefox2

Expected Results:  
To align properly like it was in firefox 2

N/A
This bug was caused by the changes in Bug 300030.
Blocks: 300030
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → 1.9.0 Branch
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
Flags: blocking1.9.1?
Flags: blocking1.9.0.2?
Keywords: regression, testcase
Version: 1.9.0 Branch → Trunk
(Reporter)

Comment 3

11 years ago
Hi Ria & Martijn

I see that the problem has been fixed, is there a download to test the fixed changes?

Thanks.

Loz
Loz, what do you mean? The problem has not been fixed yet.
(Reporter)

Comment 5

11 years ago
Hmm, odd, I thought I saw somewhere at the bottom here that it was fixed... Just looked again, didn't realise that it was something to change something to "fixed" via the drop down menu.

Sorry about that, I'm not too familiar with this site, first time reporting a bug.

Sorry for the inconvenience.

BTW, any idea how soon these patches are fixed and uploaded as another release candidate?

Cheers

Loz
Sorry, I don't know. If there's a patch and it is accepted for the 1.9.0.x branch, then there is a chance it will get into the 1.9.0.2 update, which is at least one month from now.
Is this really a bug? I assume <td align="center"> just sets text-align. But text-align doesn't actually center block-level elements. Or is this a quirk that's broken?
I suspect it's a quirk that's broken; we set text-align: -moz-center for a bunch of align attributes in HTML.
No patch for this and it's unfixed on trunk so we won't block on it, but nominating for 1.9.0.3 and marking as wanted1.9.0.x.
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.2?
Flags: blocking1.9.0.2-
(Assignee)

Comment 10

11 years ago
Created attachment 334183 [details] [diff] [review]
patch

The logic at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsHTMLReflowState.cpp&rev=1.300&mark=1922,1923,1932,1989,#1915

is flawed when a computed margin exists previous to the alignment. The margin will first reduce the marginspace that can be distributed but then it will be overwritten by the half of the remaining space so the sum of the widths will not fit the available width.
(Assignee)

Comment 11

10 years ago
Comment on attachment 334183 [details] [diff] [review]
patch

To my big surprise this went smoothly trough the regression tests. David there is nobody who knows this code better than you do you think this will fly?
Attachment #334183 - Flags: superreview?(dbaron)
Attachment #334183 - Flags: review?(dbaron)
Comment on attachment 334183 [details] [diff] [review]
patch

The change to fix mComputedMargin.left looks correct.

However, mComputedMargin.right is even worse.  We need to do a +=
for both.  So the code should actually look like:
  nscoord forLeft = availMarginSpace / 2;
  mComputedMargin.left += forLeft;
  mComputedMargin.right += (availMarginSpace - forLeft);
and there should also be a comment somewhere noting that the computed
margins need not be zero because the 'auto' could come from
overconstraint or from HTML alignment.

If you agree that all makes sense (since somebody else probably ought to 
review what I'm saying), r+sr=dbaron.
Attachment #334183 - Flags: superreview?(dbaron)
Attachment #334183 - Flags: superreview-
Attachment #334183 - Flags: review?(dbaron)
Attachment #334183 - Flags: review-
(Assignee)

Comment 13

10 years ago
Yep thats correct, I will attach a new patch soon and get it in.
(Assignee)

Comment 14

10 years ago
Created attachment 336187 [details] [diff] [review]
patch that got checked in
Attachment #334183 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Assignee: nobody → bernd_mozilla
Bernd, so this bug should be marked FIXED now that you checked in the patch?
(Assignee)

Comment 16

10 years ago
yes, tinderbox was yesterday a bit flaky (the usual weekend blues). 
<ot> why the bug resolution is on top and not close the commit button is difficult to perceive</ot>
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Flags: in-testsuite+
(In reply to comment #16)
> <ot> why the bug resolution is on top and not close the commit button is
> difficult to perceive</ot>

I guess that is bug 452888, more or less. (fwiw, I don't understand a lot of the ui changes at all).
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1?
If this didn't block the trunk it's not going to block a security update release, but you can request approval on a branch patch.
Flags: blocking1.9.0.4?
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081013 Minefield/3.1b2pre and the testcase in Comment 2.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 21

10 years ago
Hi guys,

I was wondering, have you guys found a fix yet? I've
noticed two updates since last and the problem still
persists, however, using CSS works ok.

Cheers

Loz
This is fixed on the nightly builds which will eventually lead to Firefox 3.1. You can test that it is fixed with either a nightly build from the link below or by downloading Firefox 3.1 beta 1, which should be released in a couple days.
ftp://ftp.mozilla.org/pub/firefox/nightly/2008/10/
Oh, I forgot to mention that the mozilla-central nightlies are the ones you want if you're trying a nightly.
You need to log in before you can comment on or make changes to this bug.