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

Workaround issue in Firefox 35 #2432

Merged
merged 2 commits into from
Jan 29, 2015
Merged

Workaround issue in Firefox 35 #2432

merged 2 commits into from
Jan 29, 2015

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Jan 28, 2015

In Firefox 35 we need to force the context alpha to true because setting it to false affects all framebuffers. Fixes #2431

Also removed old workaround for a Chrome issue that was fixed in Feb 2014 as well as an unused glsl #ifdef that was left over from an older workaround for Firefox.

On a side note, we should look for all of the places in the code that we have workaround and determine if they are still relevant. We should probably also start writing up issues for new workarounds that we add, similar to how we handle deprecation. This way we revisit them from time to time until they are no longer an issue.

In Firefox 35 we need to force the context alpha to true because setting
it to false affects all framebuffers. Fixes #2431

Also removed ancient workaround for a Chrome issue that was fixed in
Feb 2014 as well as an unused glsl #ifdef that was left over from an
older version of Firefox.
@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 28, 2015

we should look for all of the places in the code that we have workaround and determine if they are still relevant

+1

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 28, 2015

Sandcastle looks good. Do you get any new test failures? I went from 4 to 20.

@mramato
Copy link
Contributor Author

mramato commented Jan 29, 2015

Looks like you're right (I could have sworn I ran the tests). Other than a separate hack to avoid activating the workaround during the tests, I'm not sure what we can do. With the fix all of the Sandcastle examples seem to work, so these failures may have to be something we live with until they patch Firefox. I'm up for any ideas if people have them.

@mramato
Copy link
Contributor Author

mramato commented Jan 29, 2015

Should we also check for ANGLE and only activate it then? This is a FF Angle only issue.

@mramato
Copy link
Contributor Author

mramato commented Jan 29, 2015

@bagnell pointed out that we can't detect ANGLE before we create the context, so I'm going to just add a check for Windows instead, which I hate doing but we don't have much of a choice.

@mramato
Copy link
Contributor Author

mramato commented Jan 29, 2015

@shunter any potential issues with my Windows detection?

This should be ready unless we have any ideas on how to avoid the test failures in Firefox (which are now Windows only).

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 29, 2015

This is all good with me. I expect the failures will go away in FF 36. We can make a note in the issue to remove the workaround (perhaps even add a new label for those issues).

Tested on Windows and Mac.

@shunter please follow-up if we can improve the test.

pjcozzi added a commit that referenced this pull request Jan 29, 2015
@pjcozzi pjcozzi merged commit 8a43c4e into master Jan 29, 2015
@pjcozzi pjcozzi deleted the fix-firefox branch January 29, 2015 12:21
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

Successfully merging this pull request may close these issues.

Major regression in Firefox 35
2 participants