Gecko:Reflow Refactoring: Difference between revisions

 
(26 intermediate revisions by 5 users not shown)
Line 1: Line 1:
Landed 2006-12-07, see {{bug|300030}}
== Background and Terminology ==
== Background and Terminology ==


Line 8: Line 10:
* painting (drawing them on the screen) (the code to determine targets for events is closely connected to the painting code, so it can be considered part of this area).
* painting (drawing them on the screen) (the code to determine targets for events is closely connected to the painting code, so it can be considered part of this area).


The algorithms used to determine the positions of some types of rendering objects are described in the CSS specifications.  The most important version of the specification is CSS2.1, and the chapter with the most interesting material is 10 (visudet.html), although 9 (visuren.html), 11 (visufx.html), and 8 (box.html) are also quite relevant.  However, many of the rendering objects are described poorly (tables) or not at all (form controls, many other things) by the CSS2.1 specification.
The algorithms used to determine the positions of some types of rendering objects are described in the CSS specifications.  The most important version of the specification is [http://www.w3.org/TR/CSS21/ CSS2.1], and the chapter with the most interesting material is 10 ([http://www.w3.org/TR/CSS21/visudet.html visudet.html]), although 9 ([http://www.w3.org/TR/CSS21/visuren.html visuren.html]), 11 ([http://www.w3.org/TR/CSS21/visufx.html visufx.html]), and 8 ([http://www.w3.org/TR/CSS21/box.html box.html]) are also quite relevant.  However, many of the rendering objects are described poorly ([http://www.w3.org/TR/CSS21/tables.html tables]) or not at all (form controls, many other things) by the CSS2.1 specification.


The algorithms used to determine the positions of XUL rendering objects are architecturally rather different.  See [http://xtech06.usefulinc.com/schedule/paper/146 dbaron's XTech 2006 paper] for some further discussion of this difference.
The algorithms used to determine the positions of XUL rendering objects are architecturally rather different.  See [http://xtech06.usefulinc.com/schedule/paper/146 dbaron's XTech 2006 paper] for some further discussion of this difference.
Line 28: Line 30:


* [https://bugzilla.mozilla.org/show_bug.cgi?id=300030 Bug 300030 - Refactor intrinsic width computation out of nsIFrame::Reflow]
* [https://bugzilla.mozilla.org/show_bug.cgi?id=300030 Bug 300030 - Refactor intrinsic width computation out of nsIFrame::Reflow]
* [http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=REFLOW_200%2841213%7C50111%7C50315%7C50429%7C50804%7C51011%29_BRANCH&branchtype=regexp&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-12-13&maxdate=&cvsroot=%2Fcvsroot Bonsai Query: Reflow Branch Checkins]
* Bonsai queries
** [http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=REFLOW_200%2841213%7C50111%7C50315%7C50429%7C50804%7C51011%7C60119%7C60302%7C60603%7C60830%7C61031%29_BRANCH&branchtype=regexp&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-12-13&maxdate=&cvsroot=%2Fcvsroot All Reflow Branch checkins]
** [http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=REFLOW_20061031_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=week&mindate=&maxdate=&cvsroot=%2Fcvsroot Reflow Branch checkins in the past week]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=casesubstring&status_whiteboard=%5Breflow-refactor%5D&keywords_type=allwords&keywords=&resolution=---&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0= Bugzilla Query: Open bugs with [reflow-refactor] in status whiteboard]
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=casesubstring&status_whiteboard=%5Breflow-refactor%5D&keywords_type=allwords&keywords=&resolution=---&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0= Bugzilla Query: Open bugs with [reflow-refactor] in status whiteboard]
** some of these bugs are expected to be fixed by the work on this branch
** some of these bugs are expected to be fixed by the work on this branch
Line 36: Line 40:
** failure of the branch to fix any of these bugs deserves investigation
** failure of the branch to fix any of these bugs deserves investigation
*** there may be some classes of these bugs that will not be fixed
*** there may be some classes of these bugs that will not be fixed
* [https://bugzilla.mozilla.org/showdependencytree.cgi?id=300030&maxdepth=1&hide_resolved=1 Bugzilla Query: unfixed dependencies]
** the first set of bugs are regressions caused by this work
** the second set of bugs are unfixed bugs that can now be fixed based on this work
<!--
* [https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=casesubstring&short_desc=%5Breflow+branch%5D&product=Core&resolution=---&order=Reuse+same+sort+as+last+time Bugzilla Query: bugs with &#091;reflow branch&#093; in summary]
** these are bugs reported as ''regressions'' on the reflow branch
-->
* [ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/experimental/reflow-refactor/ Nightly Builds] (Windows only, so far)
* [http://tinderbox.mozilla.org/MozillaExperimental/ MozillaExperimental] tinderbox tree (see builds with "reflow-refactor" in the name)


== Build Instructions ==
== Build Instructions ==


<b>Note: the name of the branch has been updated, since the branch has been merged to a new branch off of current trunk.  - [[User:Dbaron|David Baron]] 14:38, 3 June 2006 (PDT)</b>
The reflow branch landed on the trunk on December 7, 2006, so the normal trunk build instructions now apply.
 
Follow the [http://developer.mozilla.org/en/docs/Build_Documentation normal build instructions] and [http://developer.mozilla.org/en/docs/Mozilla_Source_Code_Via_CVS get the source from CVS], except when checking out <code>client.mk</code>, use
 
  cvs -d :pserver:anonymous@cvs-mirror.mozilla.org:/cvsroot checkout <b>-r REFLOW_20060603_BRANCH</b> mozilla/client.mk
 
Using <code>client.mk</code> to pull the rest of the tree is essential because only layout is branched; everything other than layout is pulled by date.
 
Also, since MathML has not been converted, you must <code>--disable-mathml</code>.


== Rationale ==
== Rationale ==
Line 113: Line 118:
== Design ==
== Design ==


Parts of this section are taken from [http://groups.google.com/group/netscape.public.mozilla.layout/msg/c80282ffff8047c3 a post dbaron wrote].
Parts of this section are taken from [http://www.google.com/groups?selm=mailman.1108689676.32506.mozilla-layout@mozilla.org a post dbaron wrote].


=== Intrinsic Sizing ===
=== Intrinsic Sizing ===
Line 139: Line 144:
== Status ==
== Status ==


Currently a lot of the design is done but subject to change.  CSS block/inline layout code and XUL box layout code have been converted to the new design and work well enough that the Firefox UI basically works (although with some glitches).  However, &lt;select&gt; form controls and MathML have not yet been converted to the new design, and are disabled on the branch.
Currently a lot of the design is done but subject to change.  CSS block/inline layout code and XUL box layout code have been converted to the new design and work well enough that the Firefox UI basically works (although with some glitches).  MathML has not yet been converted to the new intrinsic width calculation design, although it does compile.


== Future Development Tasks ==
== Future Development Tasks ==


* investigate whether special height reflow quirks were introduced into standards mode for non-tables in the case where a table cell has both a table and a non-table child with percentage heights
* add [https://bugzilla.mozilla.org/show_bug.cgi?id=349296#c3 quirks] related to line breaking around replaced elements
* <s>fix incremental reflow regression on acid2 from first round of vertical resizing changes</s> - Done [[User:Dbaron|David Baron]] 17:02, 21 June 2006 (PDT)
* <s>fix incremental reflow regression on acid2 from first round of vertical resizing changes</s> - Done [[User:Dbaron|David Baron]] 17:02, 21 June 2006 (PDT)
* figure out why vertical resizing changes affected acid2 at all
* figure out why vertical resizing changes affected acid2 at all
* <s>fix display of acid2 test as -chrome (one line is wrong)</s> - Done [[User:Dbaron|David Baron]] 17:07, 21 June 2006 (PDT)
* <s>fix display of acid2 test as -chrome (one line is wrong)</s> - Done [[User:Dbaron|David Baron]] 17:07, 21 June 2006 (PDT)
* Make sure NS_FRAME_CONTAINS_RELATIVE_HEIGHT works right for quirks percentage heights
* fix marquee
* fix marquee
* consolidate move-but-don't-reflow code into nsIFrame::SlideTo and nsIFrame::SlideBy (with the code to handle the not-same-position case shared)
* consolidate move-but-don't-reflow code into nsIFrame::SlideTo and nsIFrame::SlideBy (with the code to handle the not-same-position case shared)
Line 151: Line 159:
* <s>assertions print-previewing [http://www.mozilla.com/ mozilla.com]</s> - Done [[User:Dbaron|David Baron]] 18:19, 1 July 2006 (PDT)
* <s>assertions print-previewing [http://www.mozilla.com/ mozilla.com]</s> - Done [[User:Dbaron|David Baron]] 18:19, 1 July 2006 (PDT)
* fix asserts and maybe bugs on [https://bugzilla.mozilla.org/attachment.cgi?id=226953 testcase]
* fix asserts and maybe bugs on [https://bugzilla.mozilla.org/attachment.cgi?id=226953 testcase]
* make sure the propagation of NS_FRAME_IS_DIRTY to children works on boxes, and through block->box->block


* <s>fix intrinsic sizing bug in [http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#899 default browser prompt], which has something to do with either insufficient invalidation on image load or a timing change related to the image load</s>  Done, [[User:Dbaron|David Baron]] 17:10, 6 Oct 2005 (PDT)
* <s>fix intrinsic sizing bug in [http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#899 default browser prompt], which has something to do with either insufficient invalidation on image load or a timing change related to the image load</s>  Done, [[User:Dbaron|David Baron]] 17:10, 6 Oct 2005 (PDT)
Line 159: Line 168:
** <s>implement fixed table layout</s> - Done [[User:Dbaron|David Baron]] 16:05, 24 June 2006 (PDT)
** <s>implement fixed table layout</s> - Done [[User:Dbaron|David Baron]] 16:05, 24 June 2006 (PDT)
** reconsider pixel rounding, at least in intrinsic width code
** reconsider pixel rounding, at least in intrinsic width code
** Implement fixed table layout looking at cols and colgroups.  See http://lxr.mozilla.org/seamonkey/source/layout/html/tests/formctls/bugs/bug44000.html for a testcase.
* Sort out the behavior of <code>nsHTMLReflowState::InitConstraints</code> when <code>parentReflowState</code> is null.  This happens for reflow roots and for frames being reflowed as a child of a box (via nsFrame::BoxReflow).  The current behavior seems wrong.
* fix bugs
* fix bugs
** <s>create better communication between <code>nsHTMLReflowState</code> and <code>nsTableFrame</code> about computed widths on tables so that we can do both intrinsic sizing and specified widths correctly, for both values of <code>border-collapse</code></s>, Done [[User:Dbaron|David Baron]] 01:58, 13 April 2006 (PDT)
** <s>create better communication between <code>nsHTMLReflowState</code> and <code>nsTableFrame</code> about computed widths on tables so that we can do both intrinsic sizing and specified widths correctly, for both values of <code>border-collapse</code></s>, Done [[User:Dbaron|David Baron]] 01:58, 13 April 2006 (PDT)
Line 187: Line 198:
* go through "XXX" comments added on branch and address them
* go through "XXX" comments added on branch and address them
* should table special height reflows mark dirty or ensure nsHTMLReflowState::mFlags.mVResize is set and depend on vertical resize optimizations?
* should table special height reflows mark dirty or ensure nsHTMLReflowState::mFlags.mVResize is set and depend on vertical resize optimizations?
* We can end up in a state where <code>mDirtyRoots</code> is empty after a <code>FrameNeedsReflow()</code> call -- when <code>nsGfxScrollFrameInner::LayoutScrollbars</code> sets attributes on the slider we end up in <code>nsSliderFrame::AttributeChanged</code> which calls FrameNeedsReflow().  Should we be checking that boolean that the scrollframe uses to tell itself that it's not a real curpos change?
* debug assertions related to special height reflows in gmail *compose*


== Future Testing Tasks ==
== Future Testing Tasks ==
Line 207: Line 220:
*** combine AddInlinePrefWidth and AddInlineMinWidth into one function to avoid two passes of text measurement (this could hurt the non-table cases, though, which often call GetPrefWith but not GetMinWidth)
*** combine AddInlinePrefWidth and AddInlineMinWidth into one function to avoid two passes of text measurement (this could hurt the non-table cases, though, which often call GetPrefWith but not GetMinWidth)
*** make MarkIntrinsicWidthsDirty take a child that was dirty to allow blocks to optimize per-line
*** make MarkIntrinsicWidthsDirty take a child that was dirty to allow blocks to optimize per-line
* test percentage height quirks


== Discussion and Feedback ==
== Discussion and Feedback ==
Confirmed users
489

edits