Closed Bug 752790 Opened 12 years ago Closed 7 years ago

Input padding covers text when the padding is greater than (TextHeight - InputHeight)/2

Categories

(Core :: Layout: Form Controls, defect, P2)

Product:
Core
Component:
Layout: Form Controls
Version:
12 Branch
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: gregdmathews, Assigned: MatsPalmgren_bugz)

References

(
URL
)

Details

(Whiteboard: DUPEME [webcompat] [form] [css])

Attachments

(6 files, 1 obsolete file)

Screen Shot 2012-05-07 at 8.28.18 PM.png
9.43 KB, image/png
Details
html
313 bytes, text/html
Details
firefox-left safari-right
55.24 KB, image/png
Details
input.html
646 bytes, text/html
Details
0001-Bug-752790-allow-caret-in-input-to-be-painted-over-p.patch
2.96 KB, patch
MatsPalmgren_bugz
: feedback-
Details | Diff | Splinter Review
Allow <input> content to overflow into the padding area in the block-axis (but not in the inline-axis)
1.26 KB, patch
dholbert
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

HTML:
---------------------
<html lang="en">
	<head>
		<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
		<meta charset="utf-8">
		<title>Sunshine Deals</title>
		<link href="./Sunshine Deals_files/style.css" rel="stylesheet" type="text/css">
	</head>
	<body>
   	    <form name="login" action="http://sunshine.engr.sjsu.edu/pages/db_logUser.php" method="post">
            <input value="Email Address">
            <input value="Password">
            <input class="button" type="submit" value="Submit">
   	    </form>
	</div>
</html>

CSS:
---------------------
#register input{
    height: 45px;
    padding: 18px;
}



Actual results:

The padding covers the Value text and any text that you type into the input.


Expected results:

The padding should go behind the text instead of in front of it.
This only happens in Mac OSX
The expected results are what happens in all other browsers and in Firefox for windows on Windows 7
Attached file html — 
Opera11.62 and IE10.0.8250.0CP are almost same as Firefox's behavior.
I don't think IE should be a comparison for good browser behavior. Just based on what the Windows version of Firefox does they should be consistent across operating systems. Firefox should also be consistent across input elements in how it handles the padding, as you can see in the document the input of type="submit"'s content is not overlapped by the padding. In one input element the padding overlaps the content and in another it does not. This is not expected behavior. Because padding inherits the background-color from the input field the real issue here is Should the padding be displayed in front of the content or behind the content.
This is still happening and it's inconsistent with the latest versions of other mainstream browsers.
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID 	20160113135947

Tested on Mac OS 10.7 with the Firefox Nightly 46.0a1 build. I can reproduce the problem.
On the latest version of Safari and Chrome the issue does not reproduce.
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Product: Firefox → Core
This is a duplicate...

Chrome and Safari will allow painting into the _vertical_ padding but not the _horizontal_ padding of the input, which isn't exactly expressible with CSS normally.  :(
Component: Layout → Layout: Form Controls
Whiteboard: DUPEME
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
No, this is not the same as the combobox bug, in general, though it happens to be for somewhat similar technical reasons.....
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Not sure what the real master is, but this is still an issue. So, we should align with whatever Chrome/Safari does - or?
Status: REOPENED → NEW
Another example of this issue.
See Also: → https://webcompat.com/issues/3156
Same effect on https://webcompat.com/issues/5570 for the site bestbuy.com
This is something we notice on many sites, some with high profile.
Flags: webcompat?
See Also: → https://webcompat.com/issues/3436, https://webcompat.com/issues/5570
Whiteboard: DUPEME → DUPEME [webcompat] [form] [css]
https://webcompat.com/issues/5444 etsy forms have the same issue.
Flags: webcompat? → webcompat+
Jet, what's the best way to get this issue prioritized and put on a roadmap? The "See also" bugs from webcompat.com are piling up.
Flags: needinfo?(bugs)
(In reply to Mike Taylor [:miketaylr] (55 Regression Engineering Owner) from comment #16)
> Jet, what's the best way to get this issue prioritized and put on a roadmap?
> The "See also" bugs from webcompat.com are piling up.

Is this the same as bug 1059742? Given the usage in the wild, I'm not sure we can avoid matching Blink/Webkit clipping behavior in these cases.
Flags: needinfo?(bugs) → needinfo?(mats)
Jet, yes it seems to be the same. Most of the time, the font are cut in the bottom, when in some extreme cases, the text completely disappears. My visual impression (probably making a fool of myself) is that WebKit/Blink are painting the text on top. 

See https://codepen.io/webcompat/pen/rwVdWE
and the screenshot.
Flags: needinfo?(aschen)
status-firefox57: --- → affected
Flags: needinfo?(aschen)
Priority: -- → P2
Assignee: nobody → ywu
Apparently, Chrome and Safari do paint the content area over on padding area in the vertical direction.
Not sure that if we should paint over the padding area because there are just many cases that looks *weird*.

I think, visually, for users/developers when content's background is white,
it is not clear where the boundaries are. This makes it look weird when the texts
in input are cut on FF, but it looks ok on Chrome or Safari. 

I wonder if it is possible to make an exception for "input" that we can paint the content on padding in the vertical direction?
And also, should we bring up this to discuss in css-wg?
For example, we open an issue in css-wg like 
"If it is possible to make an exception for "input" to paint the content on padding in the vertical direction?"

Hi David,
I would like to know how you will think about this.
Flags: needinfo?(mats) → needinfo?(dbaron)
I think it would be useful to bring up in the CSS Working Group once we have a concrete proposal for what the behavior should be, but I don't think it's useful to bring up for general discussion prior to that.

I think the main thing that needs to be done here is to figure out how different browser engines (Gecko, Chromium, WebKit, and Edge) behave here.  This includes looking at how they respond to changes in padding and border, whether the control uses native appearance (appearance/-webkit-appearance/-moz-appearance/etc.: none), different values of 'overflow', and perhaps even changes in font, line-height, and vertical-align.  Then, using this information, the next step is to describe in reasonable detail the behavior that is likely to give a web-compatible behavior.  Then, with that description, we can both (a) implement it and (b) bring it to the CSS Working Group.
Flags: needinfo?(dbaron)
David,

https://codepen.io/webcompat/pen/wPaweq

Here I made two boxes.
One which is fixed normal with the text (doing nothing), 
and one where I'm animating the padding so it's easier to see what is happening.

Put Safari, Chrome and Firefox side by side.

While Firefox is changing the padding vertically and horizontally 
Chrome and Safari are changing the padding ONLY horizontally (at least for the range of padding I used.)


For this to happen, we need a couple of things on the input element.

1. box-sizing: border-box;
2. a fixed height 

Things which doesn't change anything:
* border thickness
* line-height
* vertical-align
* -(moz|webkit)-appearance: none
* overflow: (visible|hidden)

The initial font-size has an influence in terms of fitting in the box but that's all. Chrome/Safari/Firefox behave the same here. Usually developers will test on Chrome on Mobile (Roboto as a fallback) and there might be issues on Firefox Android because Open Sans features are different. But that's a different issue and has no influence on this one.


Hope it helps.
Ooops and forgotten to ni on Comment #25
Flags: needinfo?(dbaron)
Hi Karl,

thanks for the test case but a small thing to add on Comment 25.
However, I think |box-sizing: border-box;| is not necessarily to be set to reproduce this issue.
I think this issue is all about the height of content area(box model) is smaller than the height of text.
For example,
you can set box-sizing to content-box(box-sizing: content-box; this should be a default setting by MDN) and set a very small height.
Then, you reproduced it.


btw I will add some browser tests soon.
Attached file input.html (obsolete) — 
This test case is easier to recognize the content area.
Attached file input.html — 
Browser tests:

Firefox, Chrome, Safari, Edge



Summary: 
(1) In the vertical direction:

when the height of the text is larger than the height of the content area.
* Firefox: Texts can only be painted into the content area. Visually, the out of the content texts are clipped.   
* Safari, Edge: Texts can be painted into the vertical padding area(top padding and bottom padding). If the top and bottom padding don’t have enough spaces, the texts are clipped on top and bottom.  
* Chrome: Texts can be painted over the content area in the vertical direction( including the top padding/border and the bottom padding/border. And texts can even be painted out of the input element's box model in the vertical direction). Visually, the texts are painted without clipping anyway.

(2) In the horizontal direction:
None of these four browsers paints the texts out of the content in the horizontal direction.

 


Things which doesn't have impacts on these four browsers:
* border thickness
* line-height
* vertical-align
* -(moz|webkit)-appearance: none
* overflow: visible/hidden/auto/scroll 
* box-sizing: No matter how we change the calculation model, it will be the same, as long as the height of the content is the same after the calculation. 



Things which change on these four browsers:
* font-size: Changing the font-size could fit the texts into the content area. 
 
 
Proposal to  bring up in css-wg:
Allow the texts in input to be painted into the top and bottom padding area. Because, visually, for users/developers when content's background color is the same as the padding’s background color, it is not clear where the boundaries of the content are.  Without clipping the texts in the vertical padding, it make the texts in input look more friendly to users. Also, with this change, this is likely to resolve the web-compatible behavior as the test results we described above.

Hi David, 
I would like to know how do you think about this?
thanks.
Attachment #8923742 - Attachment is obsolete: true
Ya-Chieh Wu, 
Thanks for the thorough tests 

I want to come back to "box-sizing: border-box;" because it has an impact 
It's the way are currently designing their sites, aka they do:

* {
  box-sizing: border-box;
}

see for example
https://css-tricks.com/box-sizing/
http://blog.teamtreehouse.com/box-sizing-secret-simple-css-layouts
https://www.paulirish.com/2012/box-sizing-border-box-ftw/



Basically take this case:

.normal {
	padding: 6px;

	border: 3px solid rgb(128, 128, 128);
	background-color: rgb(255, 192, 203);
}

input {
	box-sizing: border-box;
	height: 20px;

	background-clip: content-box;
	width: 100px;
}

as soon as the padding is 7px the text disappears, because it is "behind", no matter what is the font-size.
Hi Karl,

On Comment 30 if I understand correctly, in that case, the height of the content is 0px.
So we should not paint anything in that case.
However, as the proposal on Comment 29, if we could paint over padding, the texts can be shown.

I think what I tried to put down on Comment 29 is that no matter box-sizing is content-box or border-box,
if and only if the calculated height of the content is the same(in the case of the commment 30, we can set content-box to 0px),
it will not change the behavior in the same browser. I will put it more clearly if we decide to file an issue in css-wg.
> On Comment 30 if I understand correctly, in that case, the height of the content is 0px.
> So we should not paint anything in that case.

Yes. And the "should" is indeed correct, but it doesn't work when considering being compatible with the Web. There are usually two possible outcomes:

Case A: Either Chrome team is changing its behavior (and breaks sites. They said they were ready to do that if there was a compelling case for it to be more spec compliant)
Case B: Either Firefox changes its behavior for aligning with with Chrome/Safari implementations and modifies the specification in case it was specified.

If the spec didn't clearly explain, we need to open an issue to make it clearer or change it.

Both options have cost. 
- Case A has cost on Chrome Team and the Web developers as large.
- Case B has cost on Firefox Team (only).

There is a secondary issue related to font fallback on Android. Bug 1392147


Given the current extent of breakage and the success rate in getting sites to modify their code, Case B seems more compelling for the sake of the Web users. :/ yeah sad story.
Hi David,

I put the browser tests and a proposal to bring up this issue in css-wg on Comment 29.
Would you like to share your thoughts?
Flags: needinfo?(dbaron)
Flags: needinfo?(dbaron)
comment 29 sounds like a reasonable proposal given the research, although I think it's best to justify it based on site compatibility rather than trying to come up with reasons why it might be better.
Flags: needinfo?(dbaron)
Linking dots https://github.com/whatwg/compat/issues/81#issuecomment-342245973

> @dbaron and @bzbarsky you were both right. That box-sizing 
> wasn't the right spec, and gecko seems to get this behavior 
> more "correct".
>
> @frivoal kindly pointed me to the css-overflow spec as an 
> anchor, and I'm about to open a PR on WPT for a ref test 
> that tests for the behavior of chrome/safari/edge. I think 
> that the number of compat issues this raises makes that 
> seem like the way to go. But, I might be wrong here.
> 
> Also, this test might still not be in the right place. It 
> assumes that the relevant spec change will be to css-overflow. 
> That's the real work here, as you pointed out @bzbarsky. 
> I'm not sure what that should look like, and will look for 
> some guidance to make that happen.


[WPT] stands for Web Platform Tests
See Also: → 1404442
Ok… so… Because of bug 1404442
I found this layout.css.overflow-clip-box.enabled; which I change to true in Nightly
and then 

    overflow-clip-box: padding-box;

It doesn't exactly solve everything but it makes most of the things readable again.
(just to make it clearer, it doesn't replicate the behavior of Chrome. but makes the text overflow the padding.)
I closed my csswg-drafts issue # 1717 (https://github.com/w3c/csswg-drafts/issues/1717) in favor of Ya-Chieh Wu's issue more concise #1941 (https://github.com/w3c/csswg-drafts/issues/1941).

I also have a PR up on web-platform-tests with a reference test for the behavior that it appears chrome/edge/safari agree on in case this helps:

https://github.com/w3c/web-platform-tests/pull/8080/files
See Also: → 1419610
I suggest we make 'overflow-clip-box' a shorthand and add -block/-inline
properties, then change <input> (but not <textarea>) to overflow
the padding in the block direction only...
(In reply to Mats Palmgren (:mats) from comment #42)
> I suggest we make 'overflow-clip-box' a shorthand and add -block/-inline
> properties, then change <input> (but not <textarea>) to overflow
> the padding in the block direction only...

Can we put 'overflow-clip-box' to shorthand?
How do developer use it later? (we have to define the order of block/inline ?) 

I am thinking that maybe we can just somehow overflow the padding of <input> in block direction as default .
Per Comment 43:
How about we add one more property "overflow_clip_box_block" and keep "overflow_clip_box"?
(I am thinking that if we move "overflow_clip_box" to shorthand. we need to define the order of block/inline?
or we might change the way we use "overflow_clip_box"?)
And we set input overflow_clip_box_block: padding-box. 
This approach will add one byte into nsStyleDisplay.
Flags: needinfo?(mats)
I think I would prefer to make the changes without touching the css property of "overflow_clip_box"
since here we only want to allow the caret in input to be painted over the padding in block direction
when the overflow_clip_box is set to "content-box".
It seems that we don't really need to change "overflow_clip_box" to shorthand and add inline/block properties
for favoring this bug. (I might be wrong. I might miss something)

I think we can just hard code to change the behavior of *input*.
In the end, there is only *input* that has this difference.
Flags: needinfo?(mats)
as Comment 45, I will only make this change for *input*.

and this is very early wip. Just for someone(if any) who is interested in what I have now.
Depends on: 1422839
Comment on attachment 8934111 [details] [diff] [review]
0001-Bug-752790-allow-caret-in-input-to-be-painted-over-p.patch

I don't think this is the right way to fix this problem.
You're basically breaking 'overflow-clip-box: content-box'.

Instead, we should add the ability to specify a value
for each axis separately, and make 'overflow-clip-box'
a shorthand.  I've filed bug 1422839 to implement this.

I then intend to fix this bug by changing the initial value for
single-line form controls to:
'overflow-clip-box: padding-box content-box'.
Attachment #8934111 - Flags: feedback-
Comment on attachment 8934518 [details] [diff] [review]
Allow <input> content to overflow into the padding area in the block-axis (but not in the inline-axis)

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

r=me
Attachment #8934518 - Flags: review?(dholbert) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96a081b883af
Allow <input> content to overflow into the padding area in the block-axis (but not in the inline-axis).  r=dholbert
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/96a081b883af
Status: NEW → RESOLVED
Closed: 8 years ago → 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Thanks for fixing this bug, Mats!
We would need to get webcompat issues updated along with this fix as well.

Besides, looks like we don't add it to FF59 release note[1] yet, label it.
[1] https://www.mozilla.org/en-US/firefox/59.0a1/releasenotes/
Flags: needinfo?(kdubost)
Keywords: dev-doc-needed
I don't think putting it in release notes is necessary since it's an edge case.
Also, I don't think we should encourage this kind of layout.  Specifying
a fixed 'height' on anything that contains text is ALWAYS a bad idea.
As this is an overflow fix in form control and the behavior would be speced in css-overflow-3[1], it should be fine to release note.

[1] https://github.com/w3c/csswg-drafts/issues/1941#issuecomment-354946786
(In reply to Astley Chen (inactive) from comment #52)
> Thanks for fixing this bug, Mats!
> We would need to get webcompat issues updated along with this fix as well.
> 
> Besides, looks like we don't add it to FF59 release note[1] yet, label it.
> [1] https://www.mozilla.org/en-US/firefox/59.0a1/releasenotes/

FYI, dev-doc-needed is for MDN docs changes; we don't have anything to do with the moz.org general release notes, and we don't tend to record bugs on our developer release notes.
Keywords: dev-doc-needed → dev-doc-complete
(In reply to Astley Chen (inactive) from comment #52)
> We would need to get webcompat issues updated along with this fix as well.

Done. Verified all the open ones are fixed by this bug, and randomly tested 10 or so dupes -- all fixed as well.
Thanks Mike.
Flags: needinfo?(kdubost)
Status: RESOLVED → VERIFIED
Blocks: 1437413
See Also: → 1456052
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: