625920 - Panorama: display of the lower part of character like g, j or p (in the title tab) is cut

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P3)

Description

[Loic]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Mozilla/5.0 (Windows NT 6.0; rv:2.0b9) Gecko/20100101 Firefox/4.0b9

Latin characters with downstroke like g, j or p included a title tab are bad displayed in Panorama, in fact the lower part is missing.

Maybe the field displaying the title tab in Panorama should be larger.

Reproducible: Always




Screenshot of tabs displayed in Panorama: 
http://img3.imageshack.us/img3/6610/coupem.png

Comment 1

[Loic]

I should add I'm using the latest FF4b9 in French.

Comment 3

[Luke Iliffe (Harlequin99)]

I can confirm with the latest trunk on windows 7 with an en/us build.

STR:
1. Open minefield with a new profile.
2. Open www.google.com
3. open video.google.com
4. Open panorama

Notice how the tails of g in google titles are cut off. 

This doesn't appear to be a regression, it is present right back in mid august when panorama first appeared in m/c trunk.


Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110116 Firefox/4.0b10pre

Comment 4

[Florian J. [:FeuerFliege]]

In userChrome.css the following lines solve the problem:

.tab-title {
   line-height: 20px !important;  /*add 4px to show descenders */
   bottom: -24px !important; /* + 4px for increased line-height*/
}

If someone could change the tabview.css:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/tabview/tabview.css
136 .tab-title {
137   bottom: -20px;
138   text-align: center;
139   width: 94.5%;
140   white-space: nowrap;
141   overflow: hidden;
142   text-shadow: 0 1px rgba(255, 255, 255, 0.6);
143 }

Comment 5

[Ian Gilman [:iangilman]]

bug 567029 has just landed, which may fix this issue. Can you recheck once the nightly comes out?

Comment 6

[Florian J. [:FeuerFliege]]

No change, this bug is still alive.

Comment 7

[Florian J. [:FeuerFliege]]

Attachment: testcase with current CSS values

But I think this is just an error in the CSS:

font-size is defined as 13px, line-height is 16px. You know the total font-height is more than font-size because of internal leading in text-shadow.

I have used the current values in the testcase: everything on the red background is cut off.

Comment 8

[Florian J. [:FeuerFliege]]

sry: I meant „internal leading and text shadow“

It seems that just 2px are cut off.

So adding line-height: 18px; to http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/tabview/tabview.css#136 would fix this.

Comment 9

[Raymond Lee [:raymondlee]]

Attachment: v1

Sent it to try
http://tbpl.mozilla.org/?tree=MozillaTry&rev=7daeb50026a2

Builds and logs will be available at http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-7daeb50026a2.

Comment 10

[Raymond Lee [:raymondlee]]

Just checked.  Tab title is displayed fine for characters e.g. g, j and p on Windows XP, Mac, and Ubuntu.

Comment 11

[Raymond Lee [:raymondlee]]

(In reply to comment #10)
> Just checked.  Tab title is displayed fine for characters e.g. g, j and p on
> Windows XP, Mac, and Ubuntu.

Of course, with the patch!

Comment 12

[Ian Gilman [:iangilman]]

Comment on attachment 510239 [details] [diff] [review]
v1

thanks for fixing this!

Comment 14

[Raymond Lee [:raymondlee]]

Tested the builds on Mac, Windows XP and Ubuntu, all work fine.

Comment 15

[Dão Gottwald [:dao]]

Why are you hard-coding line-heights at all, e.g. on <body>?

Comment 16

[Raymond Lee [:raymondlee]]

(In reply to comment #15)
> Why are you hard-coding line-heights at all, e.g. on <body>?

Not sure why we have that in the .body but it fixes the problem after removing it.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 510239 [details] [diff] [review]
v1

OK, so please do that instead.

Comment 18

[Raymond Lee [:raymondlee]]

Attachment: v2

Comment 19

[Dão Gottwald [:dao]]

.titlebar also looks suspicious. I suppose removing line-height from body makes it unnecessary there as well?

Comment 20

[Raymond Lee [:raymondlee]]

Attachment: v2

Sent to try http://tbpl.mozilla.org/?tree=MozillaTry&rev=9ace1463b70e

Waiting for the builds to complete and test them.

Comment 21

[Raymond Lee [:raymondlee]]

(In reply to comment #20)
> Created attachment 511084 [details] [diff] [review]
> v2
> 
> Sent to try http://tbpl.mozilla.org/?tree=MozillaTry&rev=9ace1463b70e
> 
> Waiting for the builds to complete and test them.

Tested on Mac, Win XP and Ubuntu.

Comment 22

[Michael Yoshitaka Erlewine [:mitcho]]

Note to approvers: minor CSS patch, tests passed, virtually no risk.

Comment 23

[Dietrich Ayala (:dietrich)]

Comment on attachment 511084 [details] [diff] [review]
v2

low footprint, passed try, nice polish win -> a+.

Comment 24

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch for checkin

Comment 25

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/43a8f82dfb3d

Comment 26

[George Carstoiu]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110215 Firefox/4.0b12pre

Verified issue and it's no longer reproducible.