Closed
Bug 1140486
Opened 10 years ago
Closed 10 years ago
nsTextFrame::UpdateOverflow doesn't include the visual overflow from text measurement
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: dbaron, Assigned: dbaron)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
4.07 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
2.77 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
As I pointed out in bug 1140292 comment 1, nsTextFrame::UpdateOverflow doesn't include the visual overflow resulting from text measurement. This has been buggy since bug 524925 introduced UpdateOverflow.
This might be the fix for bug 1140292, but I'm putting it in its own bug in case it isn't.
This would mean that text frame overflow areas are insufficient whenever we've processed an UpdateOverflow style hint since the last reflow, and could lead to turds where the ink extents of the text overflow the bounding box.
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
This allows calling it from UpdateOverflow in patch 2.
Attachment #8574058 -
Flags: review?(jfkthame)
Assignee | ||
Comment 3•10 years ago
|
||
UpdateOverflow and RecomputeOverflow need to remain two separate
functions since RecomputeOverflow is called from line layout, prior to
setting the overflow areas, whereas UpdateOverflow needs to reset the
overflow areas and (via its return value) propagate the change to
ancestors. However, they should share more code than they do today.
The only substantive (non-error-handling) change this is making is that
it's adding the MeasureText call, manipulation of the resulting
boundingBox, and inclusion of that bounding box in the visual overflow.
This is exactly the change that I'm trying to make in this bug.
It's also changing the error handling if EnsureTextRun returns null, but
I don't think we need to worry about that case much.
Attachment #8574059 -
Flags: review?(jfkthame)
Updated•10 years ago
|
Attachment #8574058 -
Flags: review?(jfkthame) → review+
Updated•10 years ago
|
Attachment #8574059 -
Flags: review?(jfkthame) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0e4c63206414
https://hg.mozilla.org/mozilla-central/rev/4e6629bce17b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Assignee | ||
Comment 6•10 years ago
|
||
The question is whether there are any fuzzy-if() markings on reftests that we can remove...
You need to log in
before you can comment on or make changes to this bug.
Description
•