668919 - The ellipsis with text-overflow: ellipsis is sometimes one pixel too low

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Patrick Walton (:pcwalton)]

Screenshots of Panorama attached. Sometimes the ellipsis is one pixel too low for me, depending on the block sizing, but not always.

Comment 1

[Patrick Walton (:pcwalton)]

Attachment: Incorrect rendering for second row of tabs and below

Comment 2

[Patrick Walton (:pcwalton)]

Attachment: Correct rendering for second row of tabs and below

Comment 3

[Mats Palmgren (inactive)]

WFM in a local Linux64 mozilla-central build.  I can reproduce it on OSX
though.  A testcase would help debugging... do you know what kind of markup
are used for the text there?

Comment 4

[Patrick Walton (:pcwalton)]

Here's the CSS in question (.tab-title):

http://hg.mozilla.org/mozilla-central/file/c97034c07fe6/browser/themes/pinstripe/browser/tabview/tabview.css#l133

and

http://hg.mozilla.org/mozilla-central/file/c97034c07fe6/browser/base/content/tabview/tabview.css#l73

And here's the markup:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabview/tabitems.js#906

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 6

[Mats Palmgren (inactive)]

Testcase #1 looks fine on Linux on Windows XP.  Also when zooming up/down.

On OSX, the top three lines looks fine, on the next four the ellipsis is
too low, the bottom four looks good.

Comment 7

[Mats Palmgren (inactive)]

s/Linux on Windows/Linux and Windows/

Comment 8

[Patrick Walton (:pcwalton)]

Confirmed on Mac.

Comment 9

[Mats Palmgren (inactive)]

The problem is that nsTextFrame pixel-snaps the baseline.y before painting
the text, and we need to do the same when painting the marker text.
Should be an easy fix, just factoring out nsTextFrame::GetSnappedBaselineY
to nsLayoutUtils...

Comment 10

[Mats Palmgren (inactive)]

Attachment: fix + reftest, rev. 1

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 543626 [details] [diff] [review]
fix + reftest, rev. 1

Review of attachment 543626 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 12

[Mats Palmgren (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cdff167e8074

Comment 13

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/cdff167e8074