Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

FIx support.boxModel when padding or other styles are applied to div (#7986) #406

Closed
wants to merge 5 commits into from

4 participants

Aaron Spaulding Rick Waldron Dave Methvin Timmy Willison
Aaron Spaulding

This fixes the cases in Ticket #7986 and a case where display may not be block.

Rick Waldron
Collaborator

I've updated the ticket with this pull request http://bugs.jquery.com/ticket/7986

Unfortunately, I'm not at a machine that I can test all supported browsers - will do so tomorrow. Thanks!

Aaron Spaulding

Ok, I found some test case failures in IE 6 due to the changes that are now fixed.

test/data/support/padding.html
((24 lines not shown))
  24 + <script src="../../../src/traversing.js"></script>
  25 + <script src="../../../src/manipulation.js"></script>
  26 + <script src="../../../src/css.js"></script>
  27 + <script src="../../../src/ajax.js"></script>
  28 + <script src="../../../src/ajax/jsonp.js"></script>
  29 + <script src="../../../src/ajax/script.js"></script>
  30 + <script src="../../../src/ajax/xhr.js"></script>
  31 + <script src="../../../src/effects.js"></script>
  32 + <script src="../../../src/offset.js"></script>
  33 + <script src="../../../src/dimensions.js"></script>
  34 + </div>
  35 + <script>
  36 + window.top.supportCallback( jQuery.support );
  37 + </script>
  38 +</body>
  39 +</html>
2
Timmy Willison Collaborator
timmywil added a note

Any chance this could be integrated into an existing support page?

That shouldn't be a problem. I'll move it into bodyBackground.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Dave Methvin
Owner

@AaronAsAChimp, this pull request got stale...would you mind updating it?

Aaron Spaulding

Updated and conflict resolved.

Timmy Willison
Collaborator

Please update again with the current master and make sure that a qunit update is not committed.

Dave Methvin
Owner

I'll update this one.

Dave Methvin
Owner

Easier said than done. I spent about a half-hour trying to untangle the commits that are sprinkled in master. @AaronAsAChimp can you take a look at dmethvin@dd46db3 and see what I missed?

Aaron Spaulding

Sorry, I still have a thing or two to learn about git. It looks like you have everything.

For future reference, would git pull --rebase been cleaner to work with?

Dave Methvin
Owner

When I run the unit tests I am getting failures. I only tried Chrome but figured I must have been missing some parts of the patch. Was it passing all the unit tests for you?

Yeah, i hear you on git, i am no expert. The most important thing is to git checkout -b fix-1234-description to create a branch rather than working in master. In general I do git pull --rebase but since this was in a master branch it wasn't so happy.

Aaron Spaulding

I tried cloning your branch, but the only test case failures appear to be from QUnit not reseting the test environment properly. The failures appear to start at revision 2e5522a. I replaced qunit.js with the previous version and all of the tests passed.

Dave Methvin
Owner

@AaronAsAChimp, yes it was QUnit. I have something ready to land but would like to fix http://bugs.jquery.com/ticket/5520 at the same time since it seems related. If you have any thoughts can you add them on that ticket?

Timmy Willison
Collaborator

Please rebase with the current master and run git submodule update so that sizzle/qunit commits are not included in this pull. Also, it is preferred that pull requests are not run from master branches. This is for your convenience even more than ours. Thanks!

Aaron Spaulding

I rolled back the merges and rebased. Hopefully this is cleaner to work with.

@timmywil It would be helpful to have that information in the readme. I didn't realize how terrible of an idea it was to dev out of master until after I submitted the pull request.

Dave Methvin
Owner

We're going to deal with this in #10413, since it affects more than just $.support.boxModel and really requires more work.

Dave Methvin dmethvin reopened this
Dave Methvin
Owner

The me of 2 days ago, what a fool he was. We may want to land this by itself

Dave Methvin
Owner

Will be considered in #630, thanks @AaronAsAChimp!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
16 src/support.js
@@ -140,7 +140,10 @@ jQuery.support = (function() {
140 140 div.innerHTML = "";
141 141
142 142 // Figure out if the W3C box model works as expected
143   - div.style.width = div.style.paddingLeft = "1px";
  143 + div.style.width = div.style.padding = "1px";
  144 + div.style.border = 0;
  145 + div.style.overflow = "hidden";
  146 + div.style.display = "block";
144 147
145 148 // We don't want to do body-related feature tests on frameset
146 149 // documents, which lack a body. So we use
@@ -176,7 +179,7 @@ jQuery.support = (function() {
176 179 // value of true after appended to the DOM (IE6/7)
177 180 support.appendChecked = input.checked;
178 181
179   - support.boxModel = div.offsetWidth === 2;
  182 + support.boxModel = div.offsetWidth === 3;
180 183
181 184 if ( "zoom" in div.style ) {
182 185 // Check if natively block-level elements act like inline-block
@@ -185,13 +188,14 @@ jQuery.support = (function() {
185 188 // (IE < 8 does this)
186 189 div.style.display = "inline";
187 190 div.style.zoom = 1;
188   - support.inlineBlockNeedsLayout = ( div.offsetWidth === 2 );
  191 + support.inlineBlockNeedsLayout = ( div.offsetWidth === 3 );
189 192
190 193 // Check if elements with layout shrink-wrap their children
191 194 // (IE 6 does this)
192   - div.style.display = "";
193   - div.innerHTML = "<div style='width:4px;'></div>";
194   - support.shrinkWrapBlocks = ( div.offsetWidth !== 2 );
  195 + div.style.display = "block";
  196 + div.style.overflow = "visible";
  197 + div.innerHTML = "<div style='width:5px;'></div>";
  198 + support.shrinkWrapBlocks = ( div.offsetWidth !== 3 );
195 199 }
196 200
197 201 div.innerHTML = "<table><tr><td style='padding:0;border:0;display:none'></td><td>t</td></tr></table>";
6 test/data/support/bodyBackground.html
@@ -6,6 +6,12 @@
6 6 body {
7 7 background: #000000;
8 8 }
  9 +
  10 + div {
  11 + padding: 15px;
  12 + border: 1px solid #999;
  13 + display: inline;
  14 + }
9 15 </style>
10 16 </head>
11 17 <body>
2  test/unit/support.js
@@ -57,4 +57,4 @@ supportIFrameTest( "body background is not lost if set prior to loading jQuery (
57 57 supportIFrameTest( "A background on the testElement does not cause IE8 to crash (#9823)", "testElementCrash", function() {
58 58 expect(1);
59 59 ok( true, "IE8 does not crash" );
60   -});
  60 +});

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.