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

Already on GitHub? Sign in to your account

Streamlined and unified opacity methods #73

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

victor-homyakov commented Oct 3, 2012

Currently methods for getting/setting opacity have different and inefficient workflow:

  • in IE w/o opacity support getStyle_IE invokes getOpacity_IE (but only after useless operations like normalizeStyleName_IE())
  • in other browsers getOpacity invokes getStyle (doing useless operations like normalizeStyleName('opacity'))

Also setOpacity_IE and getOpacity_IE check STANDARD_CSS_OPACITY_SUPPORTED each time they are invoked (not good for animation performance).

Streamlined and unified opacity methods
Currently methods for getting/setting opacity have different workflow:

- in IE w/o opacity support `getStyle_IE` invokes `getOpacity_IE`
- in other browsers `getOpacity` invokes `getStyle` (doing unneeded operations like `normalizeStyleName('opacity')`)

Also `setOpacity_IE` and `getOpacity_IE` check `STANDARD_CSS_OPACITY_SUPPORTED` each time they are invoked (not good for animation performance).
Collaborator

savetheclocktower commented Nov 2, 2012

Hey, can I ask a favor? I want to apply this, but before I do, I need to make absolutely sure that this works in:

  1. IE 6 (which supports only the filter syntax)
  2. IE 7–9 (which support both filter and standard CSS opacity)
  3. IE 10 (which supports opacity but removes support for filter)

If you can assure me that the tests pass in all three of the above (for #2, testing any one of IE 7, 8, or 9 works for me), then I'll merge this. I'm very cautious about this code because I last touched it to add support for IE 10, and it was painstaking surgery to arrive at something that worked everywhere.

Contributor

victor-homyakov commented Nov 5, 2012

  1. I've ran Prototype opacity unit tests (in dom_test.html) in IE Tester (IE6, 7, 8, 9), and in standalone IE7, IE8, IE9, IE10 (on different PCs), and also in recent versions of Chrome, Firefox, Opera - OK (to be absolutely correct, I've got some test failures, e.g. in testGetElementsByClassName in IE6-7-8, testElementScrollTo in Chrome and Firefox, testElementSetStyle in Opera, testToQueryString in all browsers, many of selector_test.html in IEs, but those tests are not relevant to the subject).
  2. I've ran Scriptaculous unit and functional tests in the abovementioned browsers. Opacity in effects and draggables works fine (well, almost fine - all IEs including IE10 have smoothness/performance problems).
  3. I am using this code in production (24x7 cross-browser intranet web app) since at least 28.06.2012. Strictly speaking, I've started this code after profiling my app in IE8-9, then used it for a while in production, and only now made pull request.
Contributor

victor-homyakov commented Nov 6, 2012

There is another opacity-related issue, now in Opera: https://prototype.lighthouseapp.com/projects/8886/tickets/1282-latest-prototypejs-from-github-repo-fails-in-opera - should I make another pull request or include in this one?

Collaborator

savetheclocktower commented Nov 6, 2012

I'd say make another one. I'll merge this one soon enough.

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