Calculation of mRightFloaterCombinedArea is wrong.

RESOLVED WORKSFORME

Status

()

P3
normal
RESOLVED WORKSFORME
16 years ago
3 years ago

People

(Reporter: kinmoz, Assigned: kinmoz)

Tracking

Trunk
mozilla1.5beta
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
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|.
(Assignee)

Comment 1

16 years ago
Created attachment 99889 [details]
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.
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.2beta
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.2beta → mozilla1.3beta
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.3beta → mozilla1.5alpha
(Assignee)

Updated

16 years ago
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
Last Resolved: 3 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.