Closed Bug 365414 Opened 17 years ago Closed 17 years ago

overflowed decoration lines are not erased/painted


(Core :: Layout: Text and Fonts, defect)

Not set





(Reporter: masayuki, Assigned: masayuki)




(Keywords: jp-critical)


(3 files, 1 obsolete file)

If the decoration lines are drawn by nsHTMLContainerFrame or nsTextFrame, the overflowed decoration lines are not drawn or erased.
See the testcase of bug 212302:

The top of the link is not drawn the underline at hover state on Windows. In bug 212302, the patch fixes by the containing the decoration line to font height, but I think that the way is adhoc. So, we cannot implement the CSS3 text decoration style/width by this bug. We should calculate the overflow rect. And the clipping/erasing processes should care these area.

But I don't know that we should fix on where...
And the font of the testcase is Japanese default serif font. So, this bug is a very important for Japanese Windows users.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
I thought that the decoration lines should not affect to change the line height. But I cannot find such document in CSS2.1. If the text-decoration lines overflow from text bounding box, should we spread the bounding box? I think that the way has following merits:

1. The decoration lines never overlap to another line text, this means the users can read without the obstructive lines in all fonts.

2. The repaint area doesn't overlap with another lines, can this help the performance of dynamic style changing?

But there is a problem, if we implement the text decoration width of CSS3 text module, when the width is changed dynamic, the line-height is changed...
The content height of an inline box is somewhat undefined in CSS2.1. The line-height property is clearly defined, though, and it is either a fixed length or a multiple of the font-size, and therefore doesn't account for text decoration.

I don't think the height of the box should be affected by text-decoration, but we should be expanding the overflow rect if necessary to include it.

In terms of where the underline is positioned... I would expect it to fit within the font's bounding box. But I am not familiar with how we place underlines.

Currently, the offset + underline size may be greater than the descent of the font. It may be the issue of such fonts.

I think that by the reason (1) of comment 3, we should include the *default size* underline into the bounding box. If the size is greater than font default size, we should use overflow area. (Such case doesn't happen on current trunk.)
Attached patch Patch rv1.0 (obsolete) — Splinter Review
nsInlineFrame and nsTextFrameThebes must have the enough descent for underline.

If the gfxFont::Metrics.maxDescent always has enough size, we don't need this patch. So, we can also fix it. But it needs to change all platform's code. And same bug may be in new code (e.g., this bug). Therefore, I think that this bug should be fixed in XP level.
Assignee: nobody → masayuki
Attachment #275198 - Flags: superreview?(roc)
Attachment #275198 - Flags: review?(roc)
Could the rounding of underline offsets that we do now cause the drawing to go further than the unrounded offsets?
(In reply to comment #7)
> Could the rounding of underline offsets that we do now cause the drawing to go
> further than the unrounded offsets?

?? Sorry, I cannot understand, what you mean?
Could rounding cause us to draw outside the unrounded overflow area that you're calculating here?
er, OK. I cannot reproduce such case, but it's really in current patch... I'll create new patch, thanks.
Attached patch Patch rv2.0Splinter Review
I think that this is correct.

This patch makes that the inlineframe/textframe must have enough descent for underline. And it is rounded to nearest device pixels. nsCSSRendering::PaintDecorationLine depends on y position of the frame. But I think that the cairo clipping rounds the subpixels. Because we don't have any reports that some fonts are cut off the bottom of glyphs.

And this patch changes nsPresContext, roc, should I request to review to another people? (if you grant this patch.)
Attachment #275198 - Attachment is obsolete: true
Attachment #276262 - Flags: superreview?(roc)
Attachment #276262 - Flags: review?(roc)
Attachment #275198 - Flags: superreview?(roc)
Attachment #275198 - Flags: review?(roc)
Attachment #276262 - Flags: superreview?(roc)
Attachment #276262 - Flags: superreview+
Attachment #276262 - Flags: review?(roc)
Attachment #276262 - Flags: review+
checked-in, thanks.
Closed: 17 years ago
Resolution: --- → FIXED
> +    // Currently, only undeline is overflowable.

"underline", no?
I filed same bug at scrolling, see bug 392785.
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
You need to log in before you can comment on or make changes to this bug.