634649 - Bottom of letters such as lower-case Q, Y and G are cut off in text boxes on certain sites

Status: NEW

(Core :: Layout: Form Controls, defect)

Description

[Chris B.]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre

The bottom portion of lower-case letters, such as G, Y and Q are cut off on the above website and does not occur using Internet Explorer 9 RC.  In order to see the bug you need to log into Jibe.com using either a Facebook account or LinkedIn account and click on one of the green Apply buttons.  Then, you will be taken to a page where you can enter your contact information for the job (such as your address).

Reproducible: Always

Comment 1

[Chris B.]

Attachment: Screenshot (Minefield)

Comment 2

[Chris B.]

Attachment: Screenshot (Internet Explorer 9 RC)

Comment 3

[Ed Morley [:emorley]]

Can't confirm as it requires linking to my Facebook account in order to sign up, which I'd rather not do.

Could you see if the issue occurs with a new, empty profile:
http://support.mozilla.com/en-US/kb/Basic%20Troubleshooting#w_make-a-new-profile

Does this work fine with 3.6.13?
How about with Chrome?

(Also correcting platform, since it refers to the build of Minefield, not the OS)

Comment 4

[Chris B.]

(In reply to comment #3)
> Can't confirm as it requires linking to my Facebook account in order to sign
> up, which I'd rather not do.
> 
> Could you see if the issue occurs with a new, empty profile:
> http://support.mozilla.com/en-US/kb/Basic%20Troubleshooting#w_make-a-new-profile
> 
> Does this work fine with 3.6.13?
> How about with Chrome?
> 
> (Also correcting platform, since it refers to the build of Minefield, not the
> OS)


*** DOES NOT WORK ***
Minefield (2/17/11) - New Profile
3.6.13 - Default Profile
3.6.13 - New Profile


*** WORKS ***
Google Chrome 9.0.597.98
Internet Explorer 9.0.8080.16413 (Release Candidate)


These are all the 32-Bit versions of the browsers.

Comment 5

[Ed Morley [:emorley]]

Ok, found STR that does not require signing up.

1) Go to http://www.jibe.com/about/contact
2) Type lowercase characters with descenders (q, g, y etc) in any of the single height text boxes.

Expected:
- The descenders on each character can be seen, per IE/Chrome

Actual:
- The descenders are obscured, per screenshot attached to this bug


Confirmed with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110217 Firefox/4.0b12pre ID:20110217030357 and 3.6.13.

However not sure if this is just a CSS error made by this site? 

Marking as new for now, but might get changed to invalid once someone takes a closer look.

Comment 6

[Boris Zbarsky [:bzbarsky]]

The page explicitly sets a height of 12px on that input, and the height is too small for the text.  We clip the part of the text that's outside the set height.

I wonder whether IE allows the letters to paint in the padding area of the input or something....  That's certainly what Chrome does, as you can tell if you run this on the page in question:

  javascript:void(document.querySelectorAll("input.text")[0].style.padding = 0)

which makes Chrome render just like we do.  Note that Chrome _does_ clip the text that overlaps the left and right padding, so whatever they're doing here can't really be expressed as CSS as far as I can tell (it's overflow-x:scroll but overflow-y:visible or some such).

What does IE do here in terms of painting text over padding?

Comment 7

[(no longer active)]

Is that the right thing to do?  I thought that we're supposed to do such clipping in order to implement the box model correctly...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Which box model?

The clipping we do now is consistent with the overflow:hidden div that we put the text in, but clearly webkit is doing something different from that, right?

Comment 9

[(no longer active)]

(In reply to comment #8)
> Which box model?
> 
> The clipping we do now is consistent with the overflow:hidden div that we put
> the text in, but clearly webkit is doing something different from that, right?

Ah, ignore me.  You're right.

Do we have any other option to disable scrollbars on the DIV but don't affect the clipping?

Comment 10

[Boris Zbarsky [:bzbarsky]]

No; if you want something scrollable then we clip to the scrollable area.

And again, I don't understand the weird asymmetry in Webkit with up/down vs left/right and would like to have someone check what IE is actually doing.

Comment 11

[Mounir Lamouri (:mounir)]

(In reply to comment #6)
> The page explicitly sets a height of 12px on that input, and the height is too
> small for the text.  We clip the part of the text that's outside the set
> height.
> 
> I wonder whether IE allows the letters to paint in the padding area of the
> input or something....  That's certainly what Chrome does, as you can tell if
> you run this on the page in question:
> 
>   javascript:void(document.querySelectorAll("input.text")[0].style.padding = 0)
> 
> which makes Chrome render just like we do.  Note that Chrome _does_ clip the
> text that overlaps the left and right padding, so whatever they're doing here
> can't really be expressed as CSS as far as I can tell (it's overflow-x:scroll
> but overflow-y:visible or some such).
> 
> What does IE do here in terms of painting text over padding?

Looks like IE9 RC is doing the same thing than Webkit.

Comment 12

[Boris Zbarsky [:bzbarsky]]

I think it might be worth trying to do something like that.

Comment 13

[Chris B.]

This still occurs in the latest Nightly (7/2/11).

Comment 14

[Chris B.]

Bottom of letters are still cut off in 8/14/11 Nightly.

Comment 15

[(no longer active)]

So, Boris, is there an easy way to tell Gecko to use parent A as the scrollroot, and parent B as the clipping box?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Ehsan, I don't believe so, at the moment, but check with roc?

Comment 17

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

No.

Comment 18

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

Do we need to put the padding on the inside of the scrollroot, maybe?

Comment 19

[Boris Zbarsky [:bzbarsky]]

Would that still make the height include the padding?  If so, that might work....

Comment 20

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

I don't understand how <input> height calculations work. It seems that <input> has default box-sizing, but setting "height" definitely doesn't set the height inside the padding+borders...

Comment 21

[Boris Zbarsky [:bzbarsky]]

Sure seems to work for me.  Try this testcase:

<!DOCTYPE html>
<span style="display: inline-block; height: 20px; width: 20px; background: green; padding: 1px 0 1px 0; border: 2px solid green">a</span><input style="height: 20px; -moz-appearance: none; border-style: solid; border-color: green; vertical-align: bottom">

But I was actually talking about the auto height of the input; if people are setting heights manually bets are more or less off anyway, I think, due to differences between UAs in default stylesheets.

Comment 22

[Karl Tomlinson (:karlt)]

ISTR the behaviour of "height" depending on quirks mode.

Comment 23

[Boris Zbarsky [:bzbarsky]]

In quirks mode text inputs use border-box sizing.

Comment 24

[(no longer active)]

Attachment: WIP 1

So, does it make sense for us to just inherit the padding on the anonymous DIV, and don't set it on the text control frame itself?  Something like what this patch does...

Comment 25

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

I doubt this is enough. For one thing, doesn't it change the results of nsTextControlFrame::GetPrefSize?

Comment 26

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #25)
> I doubt this is enough. For one thing, doesn't it change the results of
> nsTextControlFrame::GetPrefSize?

Why would it?  nsBox::GetPadding gets the padding from the style information, and doesn't call GetUsedPadding, right?

Besides that, do you have any other concerns as well?

Comment 27

[Boris Zbarsky [:bzbarsky]]

The main concern on my end is how this plays with web pages trying to style the input with a particular height/border/padding.  If that works fine and auto sizing works fine, we should be good.

Comment 28

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #27)
> The main concern on my end is how this plays with web pages trying to style
> the input with a particular height/border/padding.  If that works fine and
> auto sizing works fine, we should be good.

I don't see why it shouldn't.  The only thing that this patch should change is the default 1px left/right padding for Win/IE compatibility.  We could somehow get that behavior back, but I'm not sure why that was added originally.

Comment 29

[Boris Zbarsky [:bzbarsky]]

You could look up the blame.... I seem to recall there being a good reason for it, though.

Could we not leave the left/right padding where it is now and just move the top/bottom padding into that div?

Comment 30

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

(In reply to Ehsan Akhgari [:ehsan] from comment #26)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #25)
> > I doubt this is enough. For one thing, doesn't it change the results of
> > nsTextControlFrame::GetPrefSize?
> 
> Why would it?  nsBox::GetPadding gets the padding from the style
> information, and doesn't call GetUsedPadding, right?

Right. Isn't that a problem, because the padding on the inner frame will be added to its preferred size, and then we're adding it again on the nsTextControlFrame?

> Besides that, do you have any other concerns as well?

No.

Comment 31

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #29)
> You could look up the blame.... I seem to recall there being a good reason
> for it, though.

It was added in bug 326123.  While that bug includes the rationale, it's basically "Here's what IE on Windows does, and we should copy it".

> Could we not leave the left/right padding where it is now and just move the
> top/bottom padding into that div?

Sure, why not?  :-)

Comment 32

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #30)
> (In reply to Ehsan Akhgari [:ehsan] from comment #26)
> > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #25)
> > > I doubt this is enough. For one thing, doesn't it change the results of
> > > nsTextControlFrame::GetPrefSize?
> > 
> > Why would it?  nsBox::GetPadding gets the padding from the style
> > information, and doesn't call GetUsedPadding, right?
> 
> Right. Isn't that a problem, because the padding on the inner frame will be
> added to its preferred size, and then we're adding it again on the
> nsTextControlFrame?
> 
> > Besides that, do you have any other concerns as well?
> 
> No.

OK, I'll address these comments and will submit a patch to the try server to see if I've missed something.

Comment 33

[Boris Zbarsky [:bzbarsky]]

> it's basically "Here's what IE on Windows does, and we should copy it".

I think the upshot was that it gave slightly better visual behavior...

Comment 34

[Mozilla RelEng Bot]

Try run for d31ddfce9ab6 is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=d31ddfce9ab6
Results (out of 171 total builds):
    exception: 2
    success: 107
    warnings: 46
    failure: 16
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d31ddfce9ab6

Comment 35

[(no longer active)]

This is trickier than I realized.  On Windows, we fix up the textbox paddings if it doesn't have author specified paddings: <http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsNativeThemeWin.cpp#1820>.  On Mac and Linux (GTK), the padding is not handled by the theme code.  On Linux Qt, we handle the paddings similar to Windows: <http://mxr.mozilla.org/mozilla-central/source/widget/src/qt/nsNativeThemeQt.cpp#375>

What is the best way to handle this correctly?

Comment 36

[Elbart]

Another example:
The input-field next to "Find your Device" on http://forum.xda-developers.com/showpost.php?p=53757744&postcount=3750 using the now default "XDA 2015"-forum-theme.

Comment 37

[(no longer active)]

Not sure what info you're requesting from me.  I am not working on this bug any more.

Comment 38

[pro.TaylorALove]

This S.O. post might add some additional info. Seriously though, anyone who takes on this fix will be a hero. 
http://stackoverflow.com/questions/9900018/why-is-firefox-cutting-off-the-text-in-my-input-type-text