Bug 107380 - Specifying the width/height of an input element modifies its margin
: Specifying the width/height of an input element modifies its margin
Status: UNCONFIRMED
: WebKit
CSS
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 110007
  Show dependency treegraph
 
Reported: 2013-01-19 14:25 PST by
Modified: 2013-12-24 09:37 PST (History)


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2013-01-19 14:25:40 PST
Repro steps:
1. Please http://jsfiddle.net/JZSMt/4/
2. I query getComputedStyle, and retrieve the height property of the input element without a specified height, and then set that value back in as the height style attribute value.

What happens: the marginTop and marginBottom values change from "2px" to "0px".

What I expect: the margin to remain unchanged.

This behavior deviates from FF, IE9+, and Opera, but is present in Safari 6.0 and Chrome as of this writing.
------- Comment #1 From 2013-01-19 14:37:05 PST -------
Just one more piece of information, if I query the margin, then specify the margin with the queried value, then query the height, then specify the height with the queried value, everything works as expected:

http://jsfiddle.net/JZSMt/5/
------- Comment #2 From 2013-01-20 21:02:33 PST -------
I think, this is caused by StyleResolver:

static void addIntrinsicMargins(RenderStyle* style)
{
....
    if (style->height().isAuto()) {
        if (style->marginTop().quirk())
            style->setMarginTop(Length(intrinsicMargin, Fixed));
        if (style->marginBottom().quirk())
            style->setMarginBottom(Length(intrinsicMargin, Fixed));
    }
}

and StyleResolver::adjustRenderStyle():

    if (e && e->isFormControlElement() && style->fontSize() >= 11) {
        if (!e->hasTagName(inputTag) || !static_cast<HTMLInputElement*>(e)->isI\
mageButton())
            addIntrinsicMargins(style);
    }

Since input's default height is auto and margin is 0__qem, its margin top will be "2px" (intrinsicMargin = 2). After setting input's height, e.g. 16px, the height is not auto and no addIntrinsicMargins is invoked. The marginTop is "0px".

Best regards,
Takashi Sakamoto
------- Comment #3 From 2013-01-21 05:40:42 PST -------
Thanks Takashi! So then is this a bug or expected behavior?

That seems to be the exact code path, however I'm not sure exactly WHY the code's doing that. Is there some part of the spec I'm missing that dictates this behavior?
------- Comment #4 From 2013-02-25 06:07:20 PST -------
+dhyatt

It doesn't look like anyone has touched this code since dhyatt@ checked it in, many years ago. :)

dhyatt: Would you mind giving some background on this feature? From the commit log, it appears that the purpose is to make the focus ring more visually appealing by spacing it out a bit from the element, but I can imagine that there are other reasons as well. I'm hopeful that we can find a less fragile mechanism of achieving those goals.
------- Comment #5 From 2013-02-25 11:12:43 PST -------
The purpose of intrinsic margins is to try to prevent adjacent controls from butting up against one another. Especially with rounded controls this looks terrible. The reason you see the values change is that we only set intrinsic margins when we think it won't disrupt the layout of the page. If the author specifies an explicit height/width on the control, then we assume the designer is needing pixel-precise control, and so we turn the margins off to avoid disrupting the page layout.

This feature was originally motivated by cases like Google's front page, which had two buttons that butted right up against one another. They have spacing now on the modern version of the page, but in the past they did not. This happens all over the place on the Web as well, especially with text fields next to buttons. They end up looking terrible on OS X especially (they look bad on Windows too, just not quite as bad).
------- Comment #6 From 2013-02-25 11:24:12 PST -------
From IRC (thanks, dhyatt!):

19:57 <dhyatt> mkwst: yeah i know about that code
19:57 <dhyatt> mkwst: so in order to make form controls look nice and not butt up against each other we have this feature called intrinsic margins.
19:58 <dhyatt> the basic idea is that - if we can detect it won't screw up the layout of the web page - we will add in intrinsic margins to provide a nice separation between adjacent controls the classic example years ago was google's home page with its two buttons that were right next to each other which on mac especially looked terrible since you never want aqua controls right up against one another (looks bad on windows too, just less bad).
19:59 <mkwst> dhyatt: ok. so it's not just the focus ring, but layout in general.
19:59 <dhyatt> so the basic idea is that if the author doesn't know the iwdth/height of the control (e.g., he didn't specify it explicitly) then it's safe to add in some margins but if the author said "i want this control to be 100px wide" then adding in margins is iffy since you could disrupt the author's intended page layout so that's the motivation behind sometimes turning intrinsic margins off.
20:00 <esprehn_> mkwst: form controls of full of these lies. We also tell you it has a 2 pixel beveled border and a grey background
20:00 <mkwst> esprehn_: lovely. :)
20:00 <esprehn_> getComputedStyle(input) is not super useful unless you styled it yourself
20:00 <dhyatt> the intrinsic margin feature is really nice
20:01 <dhyatt> like if you turn it off you would notice pretty fast on web pages. especially on mac.
20:01 <esprehn_> dhyatt: does Gecko do this?
20:01 <dhyatt> no this is something i invented
20:01 <dhyatt> i don't think any other browser does it
20:01 <esprehn_> seems like it would cause wrapping of controls that are inside in app toolbars. maybe that's what I've seen before.
20:01 <dhyatt> it shouldn't
20:01 <dhyatt> i mean, the margin is only added in if the control size is unspecified, so the control size was already going to vary across browsers and platforms. if the author specifies the control size, then the margins turn off 
20:03 <dhyatt> i don't recall ever seeing a bug where the intrinsic margins were to blame for disrupted page layout
20:04 <dhyatt> but if those bugs do exist, i would be willing to revisit
20:04 <esprehn_> nah, this behavior is sensible, it's just not documented
20:04 <dhyatt> but in the absence of those bugs, i'd recommend leaving the feature in, since controls with no gaps at all between them look really icky
20:04 <dhyatt> and it happens way too much
20:04 <mkwst> Right, but that bit is surprising for authors. I don't think it breaks anything in a big way, but jQuery has seen bugs filed.
20:04 <esprehn_> we could fix this with -webkit-input-spacing: auto or something
20:04 <mkwst> I don't think removing the margins is a good idea.
20:05 <esprehn_> then you'd see it in the inspector and could google search for what that was
20:05 <dhyatt> esprehn_: it needs to be margins though. if the author specifies an explicit margin, it needs to take precedence
20:05 <mkwst> But the current behavior is surprising.
20:05 <dhyatt> i mean, it's not lying. it's genuinely telling you the margin is there, right? what's surprising is that it changed.
20:06 <mkwst> That's what I mean.
20:06 <dhyatt> i mean, we could do something like not get rid of intrinsic margins if height/width are set dynamically
20:06 <dhyatt> but that seems iffy to me
20:06 <mkwst> Hrm. I agree. :/
------- Comment #7 From 2013-02-25 11:26:06 PST -------
Note that this particular test case would be mitigated completely if there was a mechanism to set and retrieve "auto" as the element's height/width.

Mike: What's the use-case you folks are running into here?
------- Comment #8 From 2013-02-25 13:08:02 PST -------
So, the use case is:

I am changing an element to "position: absolute" so I can animate its clip property. In order to do that, I need to lock in the element's width, in case it was width auto when it was relative or static and expanded to fill its container.

If the element happens to be an input, this then changes the margins of the input. I can work around this in 1 of 2 ways:
1. set the margin before setting the width, but this is less than ideal because I can't get the *computed* margin, only the used margin (from getComputedStyle), so I lose any auto-y goodness.
2. I can detect that it's an input, and assume it's got this intrinsic thing going on, but that seems like it comes with its own set of edge cases.

I currently have done #1 as my workaround.

I'll respond to the  IRC conversation in another comment. Thanks again for looking into this, guys!
------- Comment #9 From 2013-05-04 23:21:58 PST -------
@dhyatt

Hi Hyatt,

I have a further question here. Is '__qem' just designed for such use cases like preventing 'adjacent controls from butting up against one another'? Are there any other use cases in which __qem could be useful? I noticed in WebKit's user agent stylesheet, __qem is used in other element's margin settings. Are all those used to add some margin to elements while authors don't control their style precisely?
------- Comment #10 From 2013-10-17 06:21:12 PST -------
Just updating information here from Blink: it seems like a few userland bugs actually are being reported against this feature: http://code.google.com/p/chromium/issues/detail?id=128306