Setting text to empty directly inside TABLE leaves a small gap

RESOLVED FIXED

Status

()

Core
Layout: Tables
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
x86
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dbaron-1.9:RwCo])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
Created attachment 257317 [details]
372641-1a.xhtml (text in TABLE)
(Reporter)

Comment 2

10 years ago
Created attachment 257318 [details]
372641-1b.xhtml (text in TBODY)
(Reporter)

Comment 3

10 years ago
Created attachment 257319 [details]
372641-1c.xhtml (text in TR) (gap is horizontal rather than vertical)
(Reporter)

Comment 4

10 years ago
Created attachment 257320 [details]
372641-1-ref.xhtml
(Reporter)

Updated

10 years ago
Blocks: 373610
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...
Severity: minor → normal
Flags: blocking1.9?
(Reporter)

Comment 6

10 years ago
Regressed between 2007-01-29 and 2007-01-31.
Keywords: regression
Flags: blocking1.9? → blocking1.9+

Comment 7

10 years ago
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

10 years ago
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
I see a different regression range: between 2007-02-01-01 and 2007-02-02-01 ?  Nothing jumps out at me there either...
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.
Depends on: 162063
Whiteboard: [dbaron-1.9:RwCo]
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.
OK, we'll reevaluate...
Flags: blocking1.9+ → blocking1.9?
Flags: blocking1.9? → blocking1.9-
Summary: Removing text directly inside TABLE leaves a small gap → Setting text to empty directly inside TABLE leaves a small gap
Will be fixed by the patch in bug 484448.
Depends on: 484448
No longer depends on: 162063

Comment 14

8 years ago
Created attachment 373294 [details] [diff] [review]
reftests

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

Yeah, the checkin of bug 484448 fixed this issue.
Attachment #373294 - Flags: review?(bzbarsky)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
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=
Flags: in-testsuite? → in-testsuite+

Updated

8 years ago
Attachment #373294 - Attachment is obsolete: true
Attachment #373294 - Flags: review?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.