Last Comment Bug 372641 - Setting text to empty directly inside TABLE leaves a small gap
: Setting text to empty directly inside TABLE leaves a small gap
Status: RESOLVED FIXED
[dbaron-1.9:RwCo]
: regression, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on: 484448
Blocks: refdyn
  Show dependency treegraph
 
Reported: 2007-03-04 21:53 PST by Jesse Ruderman
Modified: 2009-04-17 06:51 PDT (History)
5 users (show)
roc: blocking1.9-
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
372641-1a.xhtml (text in TABLE) (263 bytes, application/xhtml+xml)
2007-03-04 21:57 PST, Jesse Ruderman
no flags Details
372641-1b.xhtml (text in TBODY) (263 bytes, application/xhtml+xml)
2007-03-04 21:57 PST, Jesse Ruderman
no flags Details
372641-1c.xhtml (text in TR) (gap is horizontal rather than vertical) (257 bytes, application/xhtml+xml)
2007-03-04 21:58 PST, Jesse Ruderman
no flags Details
372641-1-ref.xhtml (147 bytes, application/xhtml+xml)
2007-03-04 21:59 PST, Jesse Ruderman
no flags Details
reftests (2.37 KB, patch)
2009-04-17 03:58 PDT, Daniel.S
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2007-03-04 21:53:12 PST
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 2007-03-04 21:57:05 PST
Created attachment 257317 [details]
372641-1a.xhtml (text in TABLE)
Comment 2 Jesse Ruderman 2007-03-04 21:57:42 PST
Created attachment 257318 [details]
372641-1b.xhtml (text in TBODY)
Comment 3 Jesse Ruderman 2007-03-04 21:58:36 PST
Created attachment 257319 [details]
372641-1c.xhtml (text in TR) (gap is horizontal rather than vertical)
Comment 4 Jesse Ruderman 2007-03-04 21:59:04 PST
Created attachment 257320 [details]
372641-1-ref.xhtml
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2007-03-12 08:34:51 PDT
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 2007-03-12 12:36:38 PDT
Regressed between 2007-01-29 and 2007-01-31.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2007-06-05 16:31:32 PDT
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 [:bz] (still a bit busy) 2007-06-05 16:34:30 PDT
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 [:bz] (still a bit busy) 2007-10-28 22:00:04 PDT
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) 2007-10-29 00:33:57 PDT
OK, we'll reevaluate...
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2009-03-22 12:20:02 PDT
Will be fixed by the patch in bug 484448.
Comment 14 Daniel.S 2009-04-17 03:58:01 PDT
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.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2009-04-17 06:32:38 PDT
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=

Note You need to log in before you can comment on or make changes to this bug.