Closed Bug 169787 Opened 22 years ago Closed 9 years ago

Calculation of mRightFloaterCombinedArea is wrong.

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME
mozilla1.5beta

People

(Reporter: kinmoz, Assigned: kinmoz)

Details

Attachments

(1 file)

While debugging bug 159363 I learned the follwing ...

During an unconstrained reflow, FlowAndPlaceFloaters() positions all right
floaters at |mAvailSpaceRect.x|. It also prevents right floaters from being
added to the space manager, and instead tries to keep track of the areas the
right floaters would occupy in |mRightFloaterCombinedRect|. The idea is that
this would allow any content *after* the right floater to flow unconstrained and
un-impacted by any right floaters, and that after all the content for a given
line is flowed, that mRightFloaterCombinedRect could simply be added with the
combined area for the line to produce an area that was roughly equivalent to the
space needed to fit everything.

Unfortunately placing right floaters at |mAvailSpaceRect.x| makes the
calculation of the area needed by right floaters inaccurate because it allows
adjacent right floaters to be stacked on top of each other, or it's placement
impacted by an left floaters, yielding an invalid mRightFloaterCombinedRect.
This has the effect of calculating line combined widths that are either too
small or too large.

The proposed fix for bug 159363 (attachment 99589 [details] [diff] [review]) makes the sizing a bit more
consistent with the test case I will add to this bug ... but it's only
consistent because all the floaters are lined up at the unconstrained right edge.

A possible way to fix this problem, is to modify FlowAndPlaceFloaters to
position right floaters |mRightFloaterCombinedRect.x - region.width|.
Attached file Test Case
This is a testcase I've been debugging with, if you load this you'll notice how
differently some of the tables (containing left and right floaters) are sized,
though they really should be the same.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.2beta
Target Milestone: mozilla1.2beta → mozilla1.3beta
Target Milestone: mozilla1.3beta → mozilla1.5alpha
Target Milestone: mozilla1.5alpha → mozilla1.5beta
QA Contact: chrispetersen → layout
Works beautifully now.
Small question: there's a nice TC here, would it be worth it to turn it into a reftest and include it in the test suite, or is everything it covers already well tested by existing tests?
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: needinfo?(dbaron)
Resolution: --- → WORKSFORME
Maybe, but I wouldn't consider it a high priority.  It's hard to know if it's well-covered by existing tests.
Flags: needinfo?(dbaron)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: