Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mask size incorrect when target element has a different box-sizing #1273

Open
lorenzos opened this issue Jul 3, 2014 · 6 comments
Open

Mask size incorrect when target element has a different box-sizing #1273

lorenzos opened this issue Jul 3, 2014 · 6 comments

Comments

@lorenzos
Copy link
Contributor

lorenzos commented Jul 3, 2014

I'm using Mask (Spinner, really) on an Element that has box-sizing: border-box. In this case the line Mask.js:119 obviously returns an incorrect target element size because padding and border are considered when they should not. So, the mask results bigger than it should.

The issue for me is not that the default behaviour resizes the mask incorrectly, but it's the fact this behaviour cannot be changed. So, for me, the solution can be one of the following:

  1. The ['padding', 'border'] array defined in Mask.js:115 and then used in getComputedSize() can be a class option instead of hardcoded there. That way also the maskMargins option can be included in this new option, but I guess is better to leave also as it is for retro-compatibility.
  2. The same ['padding', 'border'] can be built depending on the value of this.target.getStyle('box-sizing') instead of hardcoded, but I'm not sure if this works on all browsers: box-sizing is still somewhere not supported, partially supported or need prefixes (source).

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/2942371-mask-size-incorrect-when-target-element-has-a-different-box-sizing?utm_campaign=plugin&utm_content=tracker%2F22069&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F22069&utm_medium=issues&utm_source=github).
@arian
Copy link
Member

arian commented Jul 4, 2014

Thanks. I'd say option two would be the best.

@lorenzos
Copy link
Contributor Author

lorenzos commented Jul 5, 2014

Ok, I'm going to send a pull request asap.

@lorenzos
Copy link
Contributor Author

lorenzos commented Jul 7, 2014

I had to rewrite quite the whole method (see attached PR), because using Element.getComputedSize() has unexpected behaviour in IE 9 and earlier: detecting box-sizing and changing the opt array for Element.getComputedSize() is ok when the target element is auto-sized, but fails in case of CSS-fixed width and/or height.

I noticed Element.getSize() always works well, for every box-sizing value and for both auto-size and fixed-size (tested in IE 8, 9, 10 and 11, FF and Chrome), so I tried using it.

The method looks also simpler to me now, and I'm not aware of any side-effect.

@SergioCrisostomo
Copy link
Member

@lorenzos could you make a fiddle showing the problem? I would be happy to help with the spec for this.

@lorenzos
Copy link
Contributor Author

lorenzos commented Jul 9, 2014

@SergioCrisostomo Here you are: http://jsfiddle.net/6ncT3/

To be honest I expected different results in IE9-, instead looks like the behaviour is different in all IEs. Look at comments in the JS code. I admit I'm more confused than before...

@SergioCrisostomo
Copy link
Member

I suppose this is related to IE's box-model-bug.
To fix that we need to normalize our getComputedStyle.

A broken IE spec for our getComputedStyle based on @lorenzos nice fiddle: http://jsfiddle.net/c2Xcd/

And a possible spec for the Spinner positioning (broken cross-browser): http://jsfiddle.net/57m3d/1/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants