Export to GitHub

tenfourfox - issue #84

Table layout offset to right on some websites


Posted on Aug 18, 2011 by Happy Bear

SPECIFY VERSION YOU TESTED AGAINST: FF6.0 Win PX

STR: Go to http://www.atariage.com/forums/forum/12-atari-8-bit-computers/

Tested on TFF 6.0r PowerBook G4 1333 MHz.

The Atari forum layout is correct in TFF 5.0 and FF6 on Win XP. The mess-up starts with TFF 6.0b1.

Letting TFF 6 send different user agent strings makes no difference.

The Atari forum website does not validate on http://validator.w3.org.

Trying to produce a reduced test case and re-saving the website as-is with SeaMonkey Composer produces a result that displays correctly with TFF6.

Attachments

Comment #1

Posted on Aug 18, 2011 by Happy Bear

Reduced test cases. 1) and 2) render the same in SeaMonkey 2.0.14, but differently in TFF 6.0r. The difference between 1) and 2) is one tab stroke.

Attachments

Comment #2

Posted on Aug 18, 2011 by Massive Rhino

Unfortunately the test case isn't really reduced because the CSS isn't. I'm relying on your test case to solve this since the build system is still marooned, so it needs to be something very simple. Is there a way you can break the CSS down further?

Bug 33032 looks suspicious but it doesn't specifically deal with tab characters, so I'm not sure it's the right culprit.

Comment #3

Posted on Aug 18, 2011 by Happy Bear

Someone who actually has an idea about CSS (I don't) should do this. Reducing the html is as far as I got.

Comment #4

Posted on Aug 23, 2011 by Massive Rhino

I was able to see this on the Starbucks wireless, so accepting.

Comment #5

Posted on Aug 23, 2011 by Massive Rhino

The offending style sheet of the ones it loads is http://www.atariage.com/forums/public/style_css/css_41/ipb_styles.css

Comment #6

Posted on Aug 23, 2011 by Massive Rhino

Fully minimized, independent test case. Removal of any of the tab characters restores the correct rendering. There are five in each line. Chris, confirm this appears the same for you.

Attachments

Comment #7

Posted on Aug 23, 2011 by Happy Bear

Confirmed.

Comment #8

Posted on Aug 24, 2011 by Massive Rhino

Expected rendering in 5, not in 6.0b1.

Disabling HTML5 parser doesn't change output.

No recent changes to layout that would affect this, and disabling bug 33032 doesn't change rendering. Next steps: confirm it is not in the Mac Intel 6.0, and see if it is present in 5.0.1 (if so, this would point fingers at our new font code).

Comment #9

Posted on Aug 25, 2011 by Massive Rhino

Test build of 5.0.1 shows the correct rendering. So this is a new change introduced in 6 and not our code.

Comment #10

Posted on Aug 25, 2011 by Massive Rhino

On OS X Intel 10.5 running Fx6, correct rendering is also shown.

Comment #11

Posted on Aug 30, 2011 by Massive Rhino

If you replace the entire style sheet in the test case with td,th{background-color:red} you can see what is going on: the layout actually "inserts" another cell (but it's not actually a cell because it's not red: more of an indentation). Even if you add a second cell after the (btw, regular text will do, it's not because of the image), the "phantom cell" is still there. This occurs in quirks mode also.

I tried backing out about 15 or 20 patches in isolation and got no difference in behaviour. Feel free to take a whack at the 6.0 release notes bug list and see which ones you spot that look interesting. I then tried running hg histories on layout/, parser/, gfx/ and content/, but came up with nothing obvious. This is the list I worked off: http://www.mozilla.org/en-US/firefox/6.0/releasenotes/buglist.html

The parser could be made to consume the tabs. If you insert anything else in there, anything to make the parser think it is not whitespace between the and the (like a single letter of text or another HTML container), then the tabs are correctly carried out of the table. However, I am not entirely convinced that me doing so wouldn't break something else.

Bottom line:

1) I'm out of ideas. It may not be a single patch that caused this; it could be a combination. It could even be, less likely, the compiler. 2) To deal with #1 requires me backing stuff out, one by one, from the repo and rebuilding each time until it suddenly works. This is a long process, but it will also wreck the tree. 3) I have no way of pulling down a full hg repo, just a source dump. I do not want to wreck my only existing Mozilla 6 repo until either a) Fx 7 comes out, making Fx 6 obsolete, or b) I can restore connectivity and pull down a backup copy of mozilla-release, preferably the latter. 4) Because of this, at minimum this will have to sit until I can get Fx 7 off the ground successfully or I get my T1. If you find anything yourself that looks suspicious, feel free to name it.

Comment #12

Posted on Sep 2, 2011 by Happy Rhino

First of all. Many thanks for working hard on such an excellent browser build/offshoot for the ppc. I have reduced this problem (issue 84) from my own problematic html (i.e. not from any aforementioned website) on my machine to a really small lump of html. I am running OsX 10.5 on G4/7450 with TTF 6.01. A certain sequence of white space after the "tr" seems to trip up the table rendering making the numbers "1" and "2" not in the same column. i.e. we get an unwanted right offset for row 2. My small test case is attached. Thanks.

Attachments

Comment #13

Posted on Sep 4, 2011 by Massive Rhino

I appreciate the second test case, but unfortunately it doesn't help me track down the actual problem (this is probably a symptom of something else). As soon as I can gut my Fx6 repo I plan to back out patches per comment 11.

Comment #14

Posted on Sep 26, 2011 by Massive Rhino

Replacing the tabs 1:1 with spaces will do it too. So the problem is not merely tabs.

Comment #15

Posted on Sep 26, 2011 by Massive Rhino

After several fruitless days of backing out patches, I have absolutely no idea how this broke. So I wrote up a parser bandaid that replaces the tabs with '\n' (which does not seem affected) iff the parser is currently processing a table row. This repairs the test case, but I'm debugging remotely, so I can't test it against AtariAge yet.

If this works, then we need to do a little more work. The problem also affects spaces, but we can't do a simple-minded substitution like that for spaces because it would possibly hit legitimate content. A better, though slightly slower, solution, would be to hack the whitespace checker in the parser for tables and iff the block only contains whitespace, then do the space/tab -> newline substitution.

I'll check this in more detail when I'm home from work. I really don't like doing this, but I can't think of any better solution because I absolutely can't figure out what broke and we don't have the regression builds Mozilla does because we lack the resources.

Comment #16

Posted on Sep 27, 2011 by Massive Rhino

The original site has changed, so I can't test with AtariAge. Others badly afflicted?

Comment #17

Posted on Sep 27, 2011 by Happy Bear

Look for sites using the (outdated?) IP board forum software v.3.1.4. This one http://rs2-server.org/index.php?/forum/5-announcements/ seems to be affected by the problem as well.

Comment #18

Posted on Sep 27, 2011 by Massive Rhino

That works (or rather doesn't work). I have a very narrowly scoped parser bandaid that cures all the test cases and doesn't seem to have other side effects. If it works for this URL also, then we'll ship it in the next beta.

Comment #19

Posted on Oct 3, 2011 by Massive Rhino

Internal builds of 8 show the test cases correctly, so perhaps the underlying bug was already fixed by Mozilla.

Comment #20

Posted on Oct 3, 2011 by Happy Rabbit

Interesting. I'm using Firefox 7.0.1 under 10.7.1 right now, and don't see the table problem that existed with TenFourFox 7.0 under 10.5.8. This is a weird one, but if 8 fixes it, then hallelujah! :-)

Comment #21

Posted on Oct 4, 2011 by Massive Rhino

It's an optimization bug. Debug builds don't show it, opt builds do. The problem is I don't know what component this is in.

Comment #22

Posted on Oct 8, 2011 by Massive Rhino

Dropping target since we have at least a workaround.

Comment #23

Posted on Oct 9, 2011 by Happy Rabbit

I think I found the cause for this issue!

I remembered that during my optimizations to issue 75 and 76 one time I had as a result all tables offset to the right (don't remember which one).

Now one page where this still occurs in 8.0b1 is "about:support". So I thought that if this bug is really triggered by the altivec accelerated code the G3 build shouldn't have it - and indeed with the G3 build "about:support" is rendered correctly!!!

So this one is probably a regression introduced by either issue 75 or 76.

Comment #24

Posted on Oct 9, 2011 by Happy Bear

Verified. Both about:support and the testcase from comment 1 render correctly in the G3 versions of TFF 7.0 and 8b1.

Comment #25

Posted on Oct 10, 2011 by Happy Rabbit

It's a regression from issue 75.

Comment #26

Posted on Oct 10, 2011 by Happy Rabbit

(No comment was entered for this change.)

Comment #27

Posted on Oct 11, 2011 by Massive Rhino

Nice spot on the G3 bit, but how do we know it's issue 75 and not issue 76? I'm going to look into this later today although my first priority is getting Classilla 9.2.3 shipped.

Comment #28

Posted on Oct 11, 2011 by Happy Rabbit

I already verified it is issue 75 by undef'ing TENFOURFOX_VMX in nsTextFragment.cpp . The resulting TFF build (7.0) didn't show the bug.

Comment #29

Posted on Oct 12, 2011 by Massive Rhino

(No comment was entered for this change.)

Comment #30

Posted on Oct 12, 2011 by Massive Rhino

Fair enough, blocked on 75.

Comment #31

Posted on Oct 17, 2011 by Massive Rhino

Actually, let's just dupe this to 75.

Comment #32

Posted on Nov 3, 2011 by Massive Rhino

Issue 103 has been merged into this issue.

Status: Duplicate

Labels:
Type-Defect Priority-High