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

Fix allowTransparency in the attribute table #10817

Closed
wants to merge 1 commit into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Sep 25, 2017

I’m not sure if this was intentional, but allowTransparency is not an SVG attribute.
It’s an IE-only iframe attribute. So it’s expected that Chrome wouldn’t recognize it.

Not sure if that’s useful though, as now instead of testing the wrong thing we’re not testing anything at all. 😄

| `allowTransparency=(function)`| (initial, warning, ssr mismatch)| `<null>` |
| `allowTransparency=(null)`| (initial)| `<null>` |
| `allowTransparency=(undefined)`| (initial)| `<null>` |
## `allowTransparency` (on `<div>`)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, looks like we print a container name instead of the tag name. Will send a separate fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I didn't understand your comment, but should this be (on <iframe>)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was my original intention. See #10818

Copy link
Contributor

@nhunzaker nhunzaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to do some archeology and figure out if this is actually still a thing. Quick searches aren't giving me an IE implementation matrix, but it should be pretty easy to figure out.

@nhunzaker
Copy link
Contributor

Okay, found a handy test page!

http://samples.msdn.microsoft.com/workshop/samples/author/dhtml/refs/allowTransparency.htm

I think we can remove allowTransparency. It looks like it went away with IE9.

IE7

ie7

IE8

ie8

IE9

ie9

Chrome

chrome

Safari

safari

@gaearon
Copy link
Collaborator Author

gaearon commented Sep 25, 2017

Haha let's do it then. PR?

@gaearon gaearon closed this Sep 25, 2017
@nhunzaker
Copy link
Contributor

Haha, sure thing.

@nhunzaker
Copy link
Contributor

Done in #10823

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.

None yet

3 participants