Bug Tracker

Modify

Ticket #10413 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

width, innerWidth, innerHeight, outerWidth, outerHeight are inaccurate for a "box-sizing: border-box" child of hidden parent

Reported by: mikesherov Owned by: mikesherov
Priority: low Milestone: 1.8
Component: css Version: 1.6.4
Keywords: 1.8-discuss Cc:
Blocking: #7986 Blocked by:

Description (last modified by dmethvin) (diff)

This is very similar to #9441, except this is specifically when a child (that has the box-sizing:border-box style) of a hidden div is queried.

Basically, the fallback (that uses the css width/height instead of the javascript offsetWidth/Height to calculate dimensions) is applied, and assumes that the css width doesn't contain padding and border. However, when box-sizing is border-box, the css width is the same as offsetWidth/Height, in that it accounts for padding and border.

The fix is simple, apply the same logic that is applied to offsetWidth/Height to the fallback version if boxSizing is border box.

Change History

comment:1 Changed 3 years ago by timmywil

  • Owner set to mikesherov
  • Priority changed from undecided to low
  • Status changed from new to pending
  • Component changed from unfiled to css

Thanks for taking the time to contribute to the jQuery project! Please provide a complete reduced test case on jsFiddle to help us assess your ticket.

Additionally, be sure to test against the jQuery Edge version to ensure the issue still exists. To get you started, use this boilerplate:  http://jsfiddle.net/FrKyN/ Open the link and click to "Fork" (in the top menu) to get started.

comment:2 Changed 3 years ago by anonymous

I have a patch with unit tests. Forgot to add it here:

 https://github.com/jquery/jquery/pull/528

comment:3 Changed 3 years ago by timmywil

This might be too much code for such an edge case.

comment:4 Changed 3 years ago by mikesherov

  • Status changed from pending to new

Yea, I kind of agree.

It would be a lot less code if there was already generic vendor prefix detection built into jQuery I could leverage. As far as I can tell, it's not there yet. However, that's not for me to decide when/if jQuery chooses to do that. It's a bit ironic it isn't there yet as one of the things that made me first choose jQuery was its ability to normalize cross-browser behavior.

Maybe I'll redo this bug if/when vendor prefix detection is there for me to leverage.

comment:5 Changed 3 years ago by dmethvin

  • Status changed from new to open

Yeah but a lot of people also started using jQuery because it was small ... we've got several conflicting goals at work here. If you have some idea how the reusable vendor-prefixing code might look (and how to keep it small) we'd be interested in hearing about it. We'll be starting the call for 1.8 features soon.

Related to #7986 and #5520. This box-model stuff is a pain.

comment:6 Changed 3 years ago by mikesherov

@dmethvin, don't get me wrong. Not complaining, just reflecting. I understand the whole feature creep argument, especially with a public API. Once you commit to supporting something, it's really hard to cut it later or change your mind (jquery 1.6 and Boolean attributes come to mind). Also, I agree with your code bloat argument.

The box-model stuff is a pain, but for me it's a good pain... increased flexibility in the box model means less calls for jQuery to support something like ability to set outerWidth() values.

api.jquery.com/jQuery.csshooks has a good start for vendor prefixing. Thanks for tipping me off to the feature nomination. I'll be voting for vendor prefix support in 1.8!

comment:7 Changed 3 years ago by mikesherov

@dmethvin, I went ahead and updated my pull request to include generic vendor prefixing. I tried my best to keep the functionality as small as possible.  https://github.com/jquery/jquery/pull/528

comment:8 Changed 2 years ago by mikesherov

  • Keywords 1.8-discuss added
  • Status changed from open to assigned

comment:9 Changed 2 years ago by dmethvin

  • Blocking 7986 added

comment:10 Changed 2 years ago by dmethvin

Let's be sure that the eventual solution fixes #7986.

comment:11 Changed 2 years ago by mikesherov

  • Description modified (diff)

+1, Given #10679, this is trivial!

comment:12 Changed 2 years ago by jzaefferer

  • Description modified (diff)

Why is the PR closed? Will it be reopened?

comment:13 Changed 2 years ago by jzaefferer

  • Description modified (diff)

+1

comment:14 Changed 2 years ago by jaubourg

+1

comment:15 Changed 2 years ago by dmethvin

  • Description modified (diff)

+1, Good thing I voted up on #10679 then.

comment:16 Changed 2 years ago by timmywil

-1, I don't think we should be concerned with box-sizing in dimensions. padding/border/margin are still determined manually as always.

comment:17 Changed 2 years ago by mikesherov

timmywil, it's reporting the WRONG values as per the documentation. .width says it always returns content width regardless of box-sizing. This bug is about that not being true if it's a box-sizing:border-box child.

comment:18 Changed 2 years ago by timmywil

OK, if it's the wrong value, it is a bug worth fixing. I see the point about animation as well, but .width() should continue to retrieve width regardless of box-sizing.

comment:19 Changed 2 years ago by rwaldron

+1, If it's a bug, then fix it.

comment:20 Changed 2 years ago by mikesherov

  • Milestone changed from None to 1.8

comment:21 Changed 2 years ago by mikesherov

#11565 is a duplicate of this ticket.

comment:22 Changed 2 years ago by dmethvin

  • Status changed from assigned to closed
  • Resolution set to fixed

comment:23 Changed 2 years ago by Mike Sherov

Fix #10413, #10679. Fix box-sizing:border-box and add css vendor prefix support.

Changeset: 5376a809c0d2bee4b7872847c2821e458dfdcc3b

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.