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

Remove IE8-specific shorthand style property logic #9294

Closed

Conversation

aweary
Copy link
Contributor

@aweary aweary commented Mar 30, 2017

This logic was only here for IE8. Setting .stype[prop] for shorthand properties works fine in all other tested browsers.

Testing strategy

Used the following snippet to verify shorthand property behavior

 <span id="span" style="background-color: blue; font-size: 20px; font-family: monospace; border-width: 5px; border-style: solid; border-color: red; outline-width: 10px; outline-color: green;">Click Me</span>
  <script>
    var span = document.getElementById('span');
    span.onclick = function() {
      span.style.font = '';
      span.style.background = '';
      span.style.outline = '';
      span.style.border = '';
    }
  </script>

Tested Browsers

Verified all listed browsers behaved as expected (unset the shorthand style property)

  • IE9
  • Firefox 3 (OS X Snow Leopard)
  • Safari (iOS 6)
  • Safari 4 (OS X Snow Leopard)
  • Android Browser (Android 4.1, S3)
  • Chrome (Android 4.4, Note 4)
  • Windows Phone 8.1
  • Opera 12.12

edit I updated the test to be more robust and noticed that font does not work in Android Browser for Android 4.1 or iOS 4. We may still need this if we support those browsers, though we should be able to remove a number of the properties.

@aweary
Copy link
Contributor Author

aweary commented Mar 30, 2017

Older versions of iOS and Android are giving me trouble here, so I'm closing this. Will likely open another PR with a reduced implementation of this.

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

Successfully merging this pull request may close these issues.

2 participants