Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

"Broken" Box-Model #168

Closed
dmcass opened this Issue · 21 comments

5 participants

@dmcass

I'm curious about the reasoning for using the word "broken" in the definition of this rule. While I understand that setting something like the below could cause confusion to a newcomer to css, anyone who has experience will understand how the box-model works using the default box-sizing: border-box.

/* Throws box-model warning */
.selector {
    width: 89px;
    height: 89px;
    padding: 10px;
    border: 1px solid #000;
}

By the definition of the CSS specification, this is not "broken." The padding and the border are intended to be included in the given width and height of the element, meaning that the .selector element will be 100px in width and height total.

Is the reasoning for this to remove the need for basic addition and subtraction? Or perhaps to assist newcomers in understanding how the box-model works by default?

My only issue with this rule is the wording; in the same way that the rule may assist those who are unfamiliar with the default box-model, it can hurt them by making them think that no browsers follow the spec correctly by default.

@nzakas
Owner

The intent here is to flag cases where you may end up with unintended consequences. I know I've been bitten a few times, even as a seasoned developer, by mixing width/height and padding. If you put in box-sizing, then the warning goes away because it's an indicator that you know exactly what you're doing.

Perhaps "broken" isn't the right term, and we'd certainly be open to another one. Do you have a suggestion?

@dmcass

That totally makes sense :)

Perhaps "Be aware of box-sizing" could be used? I think this accurately describes what you're trying to call out, as well as indicates what rule will silence the warning.

@mikesherov

I would add that it would be nice if this rule could distinguish between:

.selector {
    width: 89px;
    height: 89px;
    padding: 10px;
    border: 1px solid #000;
}

and

.selector {
    width: 100%;
    height: 100%;
    padding: 10px;
    border: 1px solid #000;
}

as it's clear as day that the result of the 100% example is almost NEVER what the developer intended. It should complain more loudly about the second one than the first.

@mahonnaise

I'm curious about the reasoning for using the word "broken" in the definition of this rule.

"Broken" as in "the way IE used to handle it".

Well, nowadays there are quite a lot of developers who never wrote a single line of CSS for IE5.x and most of them probably also never did anything in quirks mode. I for one haven't done either since 2003 or so. So yea, that description probably doesn't make much sense for most.

I kinda agree with mikesherov. The warning should be only triggered if percentages are used for width/height. The result of percentage based dimensions plus padding and/or border (without box-sizing) is never what you want. This would help reducing the number of false positives without undermining the core utility of this rule.

@nzakas
Owner

@mikesherov, @mahonnaise - since what you're suggesting is quite different than what this ticket is about, please file a separate issue so it can be tracked appropriately.

@mikesherov

created: #181

@stubbornella

In this case, broken just means that different browsers will handle the same code differently. That is rarely desirable. Maybe the description should be more clear about exactly which browsers are impacted?

@stubbornella stubbornella reopened this
@mikesherov

I'm curious, which browsers are impacted?

@dmcass

@stubbornella & @mikesherov,
Correct me if I'm wrong, but the only browser that will render this differently is IE6 or lower in quirks mode (@mahonnaise mentioned this as well). All modern browsers render width + padding the exact same way, and IE6 with a doctype should as well.

If the only reason for this warning is because of IE6 quirks, I would recommend removing it completely because this kind of tool doesn't appear to be catering to the least common denominator.

@mahonnaise

FWIW, I'm also in favor of replacing this rule with the rule outlined in #181. (Thanks @mikesherov! :))

While the current rule still has some relevance today, it's not very useful for most. Also, its error message is pretty confusing for everyone who doesn't know about IE6's broken box model, which can be only observed in quirks mode.

The new rule also happens to catch this particular case where someone made this simple mistake by accident or if it was done deliberately (e.g. some quirks mode IE6 only intranet application).

The difference between the two is that the new one is about unpredictable or unusable property/value combinations (i.e. something which will be always relevant) whereas the current one is about the behavior of some very old UA (i.e. something which becomes less relevant each day).

The focus/intent of the new rule is also much clearer; you don't need the whole back story if you want to explain it. It's all about very precisely defined 100% standards compliant behavior:

If you combine some percentage dimension with padding and/or border, the total size will be kinda unpredictable (and generally not what you want), if you don't also use box-sizing to switch to a different box model.

That was pretty straightforward, wasn't it? :)

@nzakas
Owner

The rule is about much more than IE6 compatibility - it's about mixing properties that may cause the box to display in an unexpected way, i.e., combining width and padding. I agree the name of the rule and the message can be clearer, but I disagree that the rule should be changed or eliminated. I think it's incredibly useful as it is, and helping people to understand its purpose should be the priority.

@dmcass

@nzakas,
Thanks for clarifying again. This was my understanding from your previous comment. I got a little bit distracted by the mention of browser inconsistencies. I'm still in favor of changing the name of the rule and keeping it, because I do think it's a good alert to potential human error.

@nzakas
Owner

So let's start brainstorming as to what the correct name for this rule is. Is it really just "Box Model Properties" or something along those lines?

@dmcass

I would suggest "Box Model Sizing," since that more accurately describes what this scenario can cause, or something similar that calls attention to size.

@stubbornella

I think we are really testing three things, and part of the murkiness may come from combining them.

  1. Box model inconsistencies if IE6 is in quirksmode
  2. W3C box model is incredibly unintuitive (most people would not expect padding and borders to add to the width)
  3. If you use box-sizing, you probably know what you are doing, but you'll have rendering differences between newer browsers and IE6&7 unless you are passing IE6/7 a different width

1 - I'd be hesitant to name this something else, since it already has a name. The internet already refers to this as the "box model bug" or says IE has a "broken box model". This should list IE6 quirksmode as the affected browser.

http://www.456bereastreet.com/archive/200612/internet_explorer_and_the_css_box_model/
http://en.wikipedia.org/wiki/Internet_Explorer_box_model_bug

2 - I frankly consider the w3c box model to be the thing that is really broken, so how do we share that advice, given that it does follow the standard? I like what PPK has to say about it:

"Logically, a box is measured from border to border. Take a physical box, any box. Put something in it that is distinctly > smaller than the box. Ask anyone to measure the width of the box. He'll measure the distance between the sides of the > box (the 'borders'). No one will think of measuring the content of the box."

All modern browsers are affected by this.

3 - is about using advanced features (box-sizing) that don't have support in two older browsers, if you are going to use it, you need to know that. This one seems like it should be called "box model sizing", it should also have IE 5.5, 6, and 7 listed as affected browsers.

@nzakas
Owner

Can we come to a conclusion on this? This rule tests for multiple things all related to the box model, so maybe just "Box Model Issues"?

@dmcass

Box Model Issues, Box Model Proprties, or Box Model Sizing are all fine with me.

If you prefer to keep Broken Box Model, I'll deal with it. I think it's misleading, but it seems there's a fundamental difference of opinions here.

@stubbornella

I guess what I was trying to say is that we need to distinguish between these three types of errors, and give a different message for each. They are all part of the box model bug family, but they are distinct.

@mikesherov

I'm going to echo @stubbornella here. I'd even go as far to say that mixing percentages and px for box model sizing should be a distinct error, but I digress ;-). cough cough #181 cough cough

@nzakas
Owner

Okay, I think we're going in circles a bit here. The intent of this bug was to say the name of the rule is misleading. The first thing that should be determined is if the rule should be renamed.

@stubbornella - We're not testing three things in this rule, we're testing just one pattern, and that is combining width/height with a box model property that affects the same dimension. Using box-sizing indicates, "hey, I know what I'm doing, leave me alone", but that really isn't a separate test. It's just a code-based way of saying disable this check for this rule.

@mikesherov - that would be a different rule

@nzakas
Owner

Note to myself: going to change the rule name to "Box Model Size" and warning to format of "Padding and borders are added to width and can sometimes make elements larger than you expect."

@nzakas nzakas closed this in 50e0f58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.