Skip to content

Commit

Permalink
Dimensions: Include scroll gutter in "padding" box
Browse files Browse the repository at this point in the history 
Fixes gh-3589
Closes gh-3656
  • Loading branch information
@gibson042
gibson042 committed Jun 19, 2017
1 parent 5bdd1ca commit 80f1c82
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 46 deletions.
114 changes: 68 additions & 46 deletions src/css.js
View file
Expand Up @@ -80,58 +80,77 @@ function setPositiveNumber( elem, value, subtract ) {
value; value;
} }


function augmentWidthOrHeight( elem, name, extra, isBorderBox, styles ) { function boxModelAdjustment( elem, dimension, box, isBorderBox, styles, computedVal ) {
var i, var i = dimension === "width" ? 1 : 0,
val = 0; extra = 0,

delta = 0;
// If we already have the right measurement, avoid augmentation
if ( extra === ( isBorderBox ? "border" : "content" ) ) { // Adjustment may not be necessary
i = 4; if ( box === ( isBorderBox ? "border" : "content" ) ) {

return 0;
// Otherwise initialize for horizontal or vertical properties
} else {
i = name === "width" ? 1 : 0;
} }


for ( ; i < 4; i += 2 ) { for ( ; i < 4; i += 2 ) {


// Both box models exclude margin, so add it if we want it // Both box models exclude margin
if ( extra === "margin" ) { if ( box === "margin" ) {
val += jQuery.css( elem, extra + cssExpand[ i ], true, styles ); delta += jQuery.css( elem, box + cssExpand[ i ], true, styles );
} }


if ( isBorderBox ) { // If we get here with a content-box, we're seeking "padding" or "border" or "margin"
if ( !isBorderBox ) {


// border-box includes padding, so remove it if we want content // Add padding
if ( extra === "content" ) { delta += jQuery.css( elem, "padding" + cssExpand[ i ], true, styles );
val -= jQuery.css( elem, "padding" + cssExpand[ i ], true, styles );
}


// At this point, extra isn't border nor margin, so remove border // For "border" or "margin", add border
if ( extra !== "margin" ) { if ( box !== "padding" ) {
val -= jQuery.css( elem, "border" + cssExpand[ i ] + "Width", true, styles ); delta += jQuery.css( elem, "border" + cssExpand[ i ] + "Width", true, styles );

// But still keep track of it otherwise
} else {
extra += jQuery.css( elem, "border" + cssExpand[ i ] + "Width", true, styles );
} }

// If we get here with a border-box (content + padding + border), we're seeking "content" or
// "padding" or "margin"
} else { } else {


// At this point, extra isn't content, so add padding // For "content", subtract padding
val += jQuery.css( elem, "padding" + cssExpand[ i ], true, styles ); if ( box === "content" ) {
delta -= jQuery.css( elem, "padding" + cssExpand[ i ], true, styles );
}


// At this point, extra isn't content nor padding, so add border // For "content" or "padding", subtract border
if ( extra !== "padding" ) { if ( box !== "margin" ) {
val += jQuery.css( elem, "border" + cssExpand[ i ] + "Width", true, styles ); delta -= jQuery.css( elem, "border" + cssExpand[ i ] + "Width", true, styles );
} }
} }
} }


return val; // Account for positive content-box scroll gutter when requested by providing computedVal
if ( !isBorderBox && computedVal >= 0 ) {

// offsetWidth/offsetHeight is a rounded sum of content, padding, scroll gutter, and border
// Assuming integer scroll gutter, subtract the rest and round down

This comment has been minimized.

Sign in to view

Copy link
@urffin

urffin Mar 21, 2019

here Assuming integer scroll gutter, subtract the rest and round down, but used Math.ceil, that round up.
seems like a bug?

This comment has been minimized.

Sign in to view

Copy link
@mgol

mgol Mar 22, 2019

Member

@urffin It's using Math.ceil but on a number from which 0.5 is subtracted. I'm not sure out of my head how that's different from Math.round but you can try it yourself & see if any tests fail. If you find ones that fail, you'll have your answer. :)

This comment has been minimized.

Sign in to view

Copy link
@urffin

urffin Mar 22, 2019 •
edited

@mgol , I just get wrong value (as i think) for table with content-box. In 3.2.1 version sum of heights rows is same as height table, but starts from 3.3.1 value is differents. Because in this point we have Math.max(0,1), and if we use floor this still was equal. Not sure should i add issue or bug for this.

This comment has been minimized.

Sign in to view

Copy link
@mgol

mgol Mar 22, 2019

Member

If you think you found an issue, by all means please report it (with a reproducible test case on jsbin.com).
delta += Math.max( 0, Math.ceil(
elem[ "offset" + dimension[ 0 ].toUpperCase() + dimension.slice( 1 ) ] -
computedVal -
delta -
extra -
0.5
) );
}

return delta;
} }


function getWidthOrHeight( elem, name, extra ) { function getWidthOrHeight( elem, dimension, extra ) {


// Start with computed style // Start with computed style
var valueIsBorderBox, var valueIsBorderBox,
styles = getStyles( elem ), styles = getStyles( elem ),
val = curCSS( elem, name, styles ), val = curCSS( elem, dimension, styles ),
isBorderBox = jQuery.css( elem, "boxSizing", false, styles ) === "border-box"; isBorderBox = jQuery.css( elem, "boxSizing", false, styles ) === "border-box";


// Computed unit is not pixels. Stop here and return. // Computed unit is not pixels. Stop here and return.
Expand All @@ -142,25 +161,28 @@ function getWidthOrHeight( elem, name, extra ) {
// Check for style in case a browser which returns unreliable values // Check for style in case a browser which returns unreliable values
// for getComputedStyle silently falls back to the reliable elem.style // for getComputedStyle silently falls back to the reliable elem.style
valueIsBorderBox = isBorderBox && valueIsBorderBox = isBorderBox &&
( support.boxSizingReliable() || val === elem.style[ name ] ); ( support.boxSizingReliable() || val === elem.style[ dimension ] );


// Fall back to offsetWidth/Height when value is "auto" // Fall back to offsetWidth/Height when value is "auto"
// This happens for inline elements with no explicit setting (gh-3571) // This happens for inline elements with no explicit setting (gh-3571)
if ( val === "auto" ) { if ( val === "auto" ) {
val = elem[ "offset" + name[ 0 ].toUpperCase() + name.slice( 1 ) ]; val = elem[ "offset" + dimension[ 0 ].toUpperCase() + dimension.slice( 1 ) ];
} }


// Normalize "", auto, and prepare for extra // Normalize "" and auto
val = parseFloat( val ) || 0; val = parseFloat( val ) || 0;


// Use the active box-sizing model to add/subtract irrelevant styles // Adjust for the element's box model
return ( val + return ( val +
augmentWidthOrHeight( boxModelAdjustment(
elem, elem,
name, dimension,
extra || ( isBorderBox ? "border" : "content" ), extra || ( isBorderBox ? "border" : "content" ),
valueIsBorderBox, valueIsBorderBox,
styles styles,

// Provide the current computed size to request scroll gutter calculation (gh-3589)
val
) )
) + "px"; ) + "px";
} }
Expand Down Expand Up @@ -319,8 +341,8 @@ jQuery.extend( {
} }
} ); } );


jQuery.each( [ "height", "width" ], function( i, name ) { jQuery.each( [ "height", "width" ], function( i, dimension ) {
jQuery.cssHooks[ name ] = { jQuery.cssHooks[ dimension ] = {
get: function( elem, computed, extra ) { get: function( elem, computed, extra ) {
if ( computed ) { if ( computed ) {


Expand All @@ -336,18 +358,18 @@ jQuery.each( [ "height", "width" ], function( i, name ) {
// in IE throws an error. // in IE throws an error.
( !elem.getClientRects().length || !elem.getBoundingClientRect().width ) ? ( !elem.getClientRects().length || !elem.getBoundingClientRect().width ) ?
swap( elem, cssShow, function() { swap( elem, cssShow, function() {
return getWidthOrHeight( elem, name, extra ); return getWidthOrHeight( elem, dimension, extra );
} ) : } ) :
getWidthOrHeight( elem, name, extra ); getWidthOrHeight( elem, dimension, extra );
} }
}, },


set: function( elem, value, extra ) { set: function( elem, value, extra ) {
var matches, var matches,
styles = extra && getStyles( elem ), styles = getStyles( elem ),
subtract = extra && augmentWidthOrHeight( subtract = extra && boxModelAdjustment(
elem, elem,
name, dimension,
extra, extra,
jQuery.css( elem, "boxSizing", false, styles ) === "border-box", jQuery.css( elem, "boxSizing", false, styles ) === "border-box",
styles styles
Expand All @@ -357,8 +379,8 @@ jQuery.each( [ "height", "width" ], function( i, name ) {
if ( subtract && ( matches = rcssNum.exec( value ) ) && if ( subtract && ( matches = rcssNum.exec( value ) ) &&
( matches[ 3 ] || "px" ) !== "px" ) { ( matches[ 3 ] || "px" ) !== "px" ) {


elem.style[ name ] = value; elem.style[ dimension ] = value;
value = jQuery.css( elem, name ); value = jQuery.css( elem, dimension );
} }


return setPositiveNumber( elem, value, subtract ); return setPositiveNumber( elem, value, subtract );
Expand Down
83 changes: 83 additions & 0 deletions test/unit/dimensions.js
View file
Expand Up @@ -544,4 +544,87 @@ QUnit.test( "width/height on an inline element with no explicitly-set dimensions
} ); } );
} ); } );


QUnit.test( "interaction with scrollbars (gh-3589)", function( assert ) {
assert.expect( 36 );

var i,
suffix = "",
updater = function( adjustment ) {
return function( i, old ) {
return old + adjustment;
};
},
parent = jQuery( "<div/>" )
.css( { position: "absolute", width: "1000px", height: "1000px" } )
.appendTo( "#qunit-fixture" ),
fraction = jQuery( "<div style='width:4.5px;'/>" ).appendTo( parent ).width() % 1,
borderWidth = 1,
padding = 2,
size = 100 + fraction,
scrollBox = {
position: "absolute",
overflow: "scroll",
width: size + "px",
height: size + "px"
},
borderBox = {
border: borderWidth + "px solid blue",
padding: padding + "px"
},
plainContentBox = jQuery( "<div />" )
.css( scrollBox )
.css( { "box-sizing": "content-box" } )
.appendTo( parent ),
contentBox = jQuery( "<div />" )
.css( scrollBox )
.css( borderBox )
.css( { "box-sizing": "content-box" } )
.appendTo( parent ),
borderBox = jQuery( "<div />" )
.css( scrollBox )
.css( borderBox )
.css( { "box-sizing": "border-box" } )
.appendTo( parent ),
$boxes = jQuery( [ plainContentBox[ 0 ], contentBox[ 0 ], borderBox[ 0 ] ] );

for ( i = 0; i < 3; i++ ) {
if ( i === 1 ) {
suffix = " after increasing inner* by " + i;
size += i;
$boxes.innerWidth( updater( i ) ).innerHeight( updater( i ) );
} else if ( i === 2 ) {
suffix = " after increasing outer* by " + i;
size += i;
$boxes.outerWidth( updater( i ) ).outerHeight( updater( i ) );
}

assert.equal( plainContentBox.innerWidth(), size,
"plain content-box innerWidth includes scroll gutter" + suffix );
assert.equal( plainContentBox.innerHeight(), size,
"plain content-box innerHeight includes scroll gutter" + suffix );
assert.equal( plainContentBox.outerWidth(), size,
"plain content-box outerWidth includes scroll gutter" + suffix );
assert.equal( plainContentBox.outerHeight(), size,
"plain content-box outerHeight includes scroll gutter" + suffix );

assert.equal( contentBox.innerWidth(), size + 2 * padding,
"content-box innerWidth includes scroll gutter" + suffix );
assert.equal( contentBox.innerHeight(), size + 2 * padding,
"content-box innerHeight includes scroll gutter" + suffix );
assert.equal( contentBox.outerWidth(), size + 2 * padding + 2 * borderWidth,
"content-box outerWidth includes scroll gutter" + suffix );
assert.equal( contentBox.outerHeight(), size + 2 * padding + 2 * borderWidth,
"content-box outerHeight includes scroll gutter" + suffix );

assert.equal( borderBox.innerWidth(), size - 2 * borderWidth,
"border-box innerWidth includes scroll gutter" + suffix );
assert.equal( borderBox.innerHeight(), size - 2 * borderWidth,
"border-box innerHeight includes scroll gutter" + suffix );
assert.equal( borderBox.outerWidth(), size,
"border-box outerWidth includes scroll gutter" + suffix );
assert.equal( borderBox.outerHeight(), size,
"border-box outerHeight includes scroll gutter" + suffix );
}
} );

} )(); } )();

0 comments on commit 80f1c82

Please sign in to comment.