512487 - reftest first-letter/399941-8.html regressed on Linux (possible Pango 1.24 issue)

Status: RESOLVED FIXED

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

Description

[Zack Weinberg (:zwol)]

Attachment: screen shot of regression -- yellow pixels are wrong

Reftest 399941-8.html has regressed for me on Linux.  I'm attaching a screen shot of the rendering with differences highlighted -- the olive background on the rightmost Phoenician letter is drawn one pixel bigger on top and bottom than the reference.

At a guess, this changed with very recent Pango (I have 1.24).

Comment 1

[Mike Hommey [:glandium]]

Also applies on 1.9.2, and most probably trunk

Comment 2

[Mike Hommey [:glandium]]

I can confirm the test passes with pango 1.20 (from debian stable)

Comment 3

[Mike Hommey [:glandium]]

(In reply to comment #2)
> I can confirm the test passes with pango 1.20 (from debian stable)

Actually I wasn't testing correctly with the older pango. It happens to depend on installed font: if no font supports phoenician, the result, which includes the square replacement characters, matches reference, but doesn't otherwise, both with pango 1.26 and 1.20.

Comment 4

[Mike Hommey [:glandium]]

fwiw, i was testing with the freefont font.

Comment 5

[Zack Weinberg (:zwol)]

There's no fancy text shaping applied to the Phoenician characters in FreeSans.ttf as far as I can tell, so this suggests to me that line height calculations are being done differently for ::first-letter than for the fake (an explicit <span> surrounding the first letter), possibly only when right-to-left text is involved.  cc:ing the usual suspects.

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to comment #5)
> There's no fancy text shaping applied to the Phoenician characters in
> FreeSans.ttf as far as I can tell, so this suggests to me that line height
> calculations are being done differently for ::first-letter than for the fake
> (an explicit <span> surrounding the first letter), possibly only when
> right-to-left text is involved.  cc:ing the usual suspects.

I think there's intentional behavior of that sort for floating :first-letter, but I don't think that should happen for non-floating :first-letter, although it's possible it does because there's an nsFirstLetterFrame in there.

Comment 7

[Mike Hommey [:glandium]]

I don't know if this is intended or related, but there is also an horizontal difference when adding a border to the style.

Comment 8

[Simon Montagu :smontagu]

See also bug 454963. 

If I am not mistaken, styling the background is not an essential part of this test (though it makes the results easier to check for human eyes), so we could just not do that.

Does that make sense? I think it's isolating the problem rather than avoiding it, but maybe people disagree?

Comment 9

[Mike Hommey [:glandium]]

I think it would make sense to keep the first-letter test do first-letter checking only. Replacing the background color with a text color should work just as fine for that purpose, I guess.
But then, another test should be written for the background color test.

Comment 10

[Simon Montagu :smontagu]

(In reply to comment #5)
> There's no fancy text shaping applied to the Phoenician characters in
> FreeSans.ttf as far as I can tell, so this suggests to me that line height
> calculations are being done differently for ::first-letter than for the fake
> (an explicit <span> surrounding the first letter), possibly only when
> right-to-left text is involved.  cc:ing the usual suspects.

FWIW from the cases reported as failing in bug 454963 I suspect this is an issue with supplementary characters rather than right-to-left. Does anyone seeing the bug with 399941-8.html also see it with 399941-9.html (assuming that you have a font with cuneiform characters)?

Comment 11

[Simon Montagu :smontagu]

(In reply to comment #9)
> But then, another test should be written for the background color test.

Agreed, though it will probably have to use a downloadable font.

Comment 12

[Simon Montagu :smontagu]

(In reply to comment #10)
> Does anyone
> seeing the bug with 399941-8.html also see it with 399941-9.html (assuming that
> you have a font with cuneiform characters)?

To answer my own question, I see the bug myself with 399941-9.html after installing CuneiformComposite.ttf from http://cdl.museum.upenn.edu/cuneifonts.html

Comment 13

[Zack Weinberg (:zwol)]

We should maybe be using downloadable fonts for these tests anyway so that we get consistent behavior.

Comment 14

[Mike Hommey [:glandium]]

(In reply to comment #10)
> Does anyone seeing the bug with 399941-8.html also see it with 399941-9.html (assuming that
> you have a font with cuneiform characters)?

Not me. Here, only 399941-8.html fails.

Comment 15

[Mike Hommey [:glandium]]

Attachment: Patch for first-letter/399941-8.html and first-letter/399941-9.html

As per first half of comment 9. The second half may be subject of another bug, I guess.

Comment 16

[Mike Hommey [:glandium]]

> Not me. Here, only 399941-8.html fails.

I don't have a font implementing cuneiform characters, which is why only -8 fails for me.

Comment 17

[Zack Weinberg (:zwol)]

Pushed http://hg.mozilla.org/mozilla-central/rev/46f0ec7e2d52 .  I'll see if I can't make a new version of the test case with downloadable fonts, so we can get the background-color bug fixed too.