372641 - Setting text to empty directly inside TABLE leaves a small gap

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

If there is some text directly inside a TABLE (or TBODY or TR), and I remove it by setting the text node's data to the empty string, a small gap remains, leaving the table asymmetric.  I will attach reftests.

Comment 1

[Jesse Ruderman]

Attachment: 372641-1a.xhtml (text in TABLE)

Comment 2

[Jesse Ruderman]

Attachment: 372641-1b.xhtml (text in TBODY)

Comment 3

[Jesse Ruderman]

Attachment: 372641-1c.xhtml (text in TR) (gap is horizontal rather than vertical)

Comment 4

[Jesse Ruderman]

Attachment: 372641-1-ref.xhtml

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hmm.  The immediate problem is that those pseudo-frames are not being removed, but we shouldn't be creating those pseudo-frames at all, should we?  See the NeedFrameFor() check in nsCSSFrameConstructor::ConstructFrame.  Bernd, any idea what's up here?

Also, is this a regression?  I rather suspect it is...

Comment 6

[Jesse Ruderman]

Regressed between 2007-01-29 and 2007-01-31.

Comment 7

[Bernd]

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Flayout&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-29&maxdate=2007-01-31&cvsroot=%2Fcvsroot

Comment 8

[Bernd]

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Flayout&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-29&maxdate=2007-02-01&cvsroot=%2Fcvsroot

Comment 9

[Boris Zbarsky [:bzbarsky]]

I see a different regression range: between 2007-02-01-01 and 2007-02-02-01 ?  Nothing jumps out at me there either...

Comment 10

[Boris Zbarsky [:bzbarsky]]

Oh, wait.  NeedFrameFor only triggers on whitespace-only text.  So we do expect pseudos here, and this is basically bug 162063 in a different form.

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK.  I see what happened here.

On branch, the <script> runs before we ever start layout, so the frame model doesn't have to deal with that dynamic change.

On trunk, we started doing incremental XML layout.  So now we're flushing out the layout for the offsetWidth get, then it has to update dynamically in response to the text set and fails.  Of course in HTML, this has been a problem all along.

As far as the regression range goes, the range I'm getting includes the fix for bug 369011.  Until that fix, flushing the XML content sink randomly didn't work (because sometimes it thought that it was in the middle of a notification when it actually wasn't).  I still have no idea where Jesse's range comes from, other than the likely "something changed what value happened to be at that uninitialized memory location" reason.

I don't think this bug should be a blocker.  Especially because it's not even clear what the right layout of this sort of markup is.  I posted to www-style about that; no response so far other than the "it's been added to the issues list" boilerplate.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, we'll reevaluate...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Will be fixed by the patch in bug 484448.

Comment 14

[Daniel.S]

Attachment: reftests

(In reply to comment #13)
> Will be fixed by the patch in bug 484448.

Yeah, the checkin of bug 484448 fixed this issue.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Oh, I totally forgot to close this; thank you for noticing that!  I landed tests for this as part of the patch in bug 484448.  See http://mxr.mozilla.org/mozilla-central/find?string=372641&tree=mozilla-central&hint=