Last Comment Bug 713856 - Dynamic change of DOM in lists before block element creates blank line
: Dynamic change of DOM in lists before block element creates blank line
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla12
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
: 697793 719360 735554 (view as bug list)
Depends on:
Blocks: 389321
  Show dependency treegraph
 
Reported: 2011-12-28 06:45 PST by Steef389
Modified: 2012-12-05 06:42 PST (History)
10 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (278 bytes, text/html)
2011-12-28 06:45 PST, Steef389
no flags Details
static testcase (126 bytes, text/html)
2011-12-29 15:55 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
no flags Details
Patch (v1) (2.86 KB, patch)
2011-12-30 10:10 PST, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review

Description Steef389 2011-12-28 06:45:33 PST
Created attachment 584562 [details]
Testcase

If there is a newline in a list before a block element it gets stripped off.

What to do:
Change the textelement containing the newline with javascript to the same content.

Excepted result:
No change in display, newline should not be shown.

Actual result:
Newline is shown in rendered page.


May be relate to Bug 697793 and Bug 690164
Comment 1 Alice0775 White 2011-12-28 08:37:24 PST
Regression window:
Works;
http://hg.mozilla.org/mozilla-central/rev/c0dbdafa583c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre ID:20101104134944
Fails:
http://hg.mozilla.org/mozilla-central/rev/52a6a18eeb61
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre ID:20101104165457
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c0dbdafa583c&tochange=52a6a18eeb61

Triggered by:
ed0befc22bb7	Ehsan Akhgari — Bug 389321 - Part 3: Use a centralized algorithm for caret positioning; r=roc a=blocking-betaN+
Comment 2 Boris Zbarsky [:bz] 2011-12-28 12:17:07 PST
So presumably the only reason the newline doesn't show up the first time is due to the frame-construction-time whitespace frame suppression?
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-12-28 13:34:02 PST
My guess (untested) would be that this is another regression from bug 389321, and likely a duplicate of one of the existing regressions from it.
Comment 4 Boris Zbarsky [:bz] 2011-12-28 13:45:16 PST
> My guess (untested) would be that this is another regression from bug 389321,

Comment 1 says so too, based on a bisect.
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-29 15:24:50 PST
This seems to be the code responsible for this bug: http://hg.mozilla.org/mozilla-central/rev/ed0befc22bb7#l15.12
Comment 6 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-29 15:29:59 PST
(In reply to Boris Zbarsky (:bz) from comment #2)
> So presumably the only reason the newline doesn't show up the first time is
> due to the frame-construction-time whitespace frame suppression?

I think this is the reason, since the code in question doesn't get run the first time that the document is reflown.  Can you please point me to the code responsible for the whitespace frame suppression?  Why don't we do the same thing on the future reflows?
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-29 15:42:20 PST
It's an optimization that happens at frame construction time. When you assign to the node data, we undo the optimization ... it's simpler than trying to keep it optimized through dynamic changes.
Comment 8 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-29 15:45:20 PST
Do we undo the optimization only for the textframe that changed?  Note that in this case, the textframe before the <p> element (containing a "\n") triggers this bug.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-29 15:51:12 PST
(In reply to Ehsan Akhgari [:ehsan] from comment #8)
> Do we undo the optimization only for the textframe that changed?

Yes. The optimization really has nothing to do with this though.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-29 15:55:15 PST
Created attachment 584875 [details]
static testcase

Here's a testcase where the empty-textframe optimization never kicks in (because the whitespace-only text node between the <li> and <p> is not adjacent to a block element boundary), which shows the bug.
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-30 10:10:40 PST
Created attachment 584982 [details] [diff] [review]
Patch (v1)

Oh, I figured out the problem.  This change had snuck its way into the patch for bug 389321.  It's not needed, and reverting it fixes this bug.
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-30 10:19:37 PST
*** Bug 697793 has been marked as a duplicate of this bug. ***
Comment 13 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-30 14:30:24 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/36cd5cc8a909
Comment 14 Mozilla RelEng Bot 2011-12-30 16:50:36 PST
Try run for 33568bb15cbb is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=33568bb15cbb
Results (out of 222 total builds):
    exception: 5
    success: 187
    warnings: 26
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-33568bb15cbb
 Timed out after 06 hours without completing.
Comment 15 Phil Ringnalda (:philor) 2011-12-31 19:35:43 PST
https://hg.mozilla.org/mozilla-central/rev/36cd5cc8a909
Comment 16 Boris Zbarsky [:bz] 2012-01-19 07:49:45 PST
*** Bug 719360 has been marked as a duplicate of this bug. ***
Comment 17 :Ehsan Akhgari (busy, don't ask for review please) 2012-03-14 15:57:26 PDT
*** Bug 735554 has been marked as a duplicate of this bug. ***

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