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 #12617: throw Blob() support errors #12666

Closed
wants to merge 1 commit into from
Closed

Fix #12617: throw Blob() support errors #12666

wants to merge 1 commit into from

Conversation

zlatanvasovic
Copy link
Contributor

This should work in all cases per FileSaver.js docs.
Fix #12617.

@Merg1255
Copy link

Track this issue with: eligrey/FileSaver.js#12

@XhmikosR
Copy link
Member

Makes sense until this is fixed upstream somehow. Will test tomorrow with old IE.

@zlatanvasovic
Copy link
Contributor Author

@XhmikosR Thanks.

@ssorallen
Copy link
Contributor

This won't change the situation for Safari 6, 6.1, or 7, which all implement Blob but do not support opening blob: URLs. I tested Safari 7 locally and Safari 6 and 6.1 on BrowserStack.

for HTML5 blobs. Because of this your file will be downloaded with the name <code>"untitled"</code>.\
However, if you check your downloads folder, just rename this <code>"untitled"</code> file\
to <code>"bootstrap.zip"</code> and you should be good to go!')
} else if (!window.URL && !window.webkitURL) {
} else if (!window.URL && !window.webkitURL || !new Blob()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add parens to clarify operator binding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they may help with code readability, I'll add them.

Note: they won't change code functionallity. :)

2014-02-13 0:19 GMT+01:00 Chris Rebert [email protected]:

In docs/assets/js/customizer.js:

              for HTML5 blobs. Because of this your file will be downloaded with the name <code>"untitled"</code>.\
              However, if you check your downloads folder, just rename this <code>"untitled"</code> file\
              to <code>"bootstrap.zip"</code> and you should be good to go!')
  • } else if (!window.URL && !window.webkitURL) {
  • } else if (!window.URL && !window.webkitURL || !new Blob()) {

maybe add parens to clarify operator binding?


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666/files#r9690993
.

Zlatan Vasović - ZDroid

@zlatanvasovic
Copy link
Contributor Author

@ssorallen Did you find a way to prevent that? I didn't find anything like window.blobURL. 😟

@cvrebert
Copy link
Collaborator

We'll most likely have to use UA sniffing or similar to exclude Safari & older IE.
Anyway, fix the paren thing and exclude customize.min.js, and we'll merge this in ASAP.

@zlatanvasovic
Copy link
Contributor Author

Why to exclude customize.min.js?

2014-02-13 8:14 GMT+01:00 Chris Rebert [email protected]:

We'll most likely have to use UA sniffing or similar to exclude Safari &
older IE.
Anyway, fix the paren thing and exclude customize.min.js, and we'll merge
this in ASAP.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666#issuecomment-34953726
.

Zlatan Vasović - ZDroid

@cvrebert
Copy link
Collaborator

Avoids merge conflicts.

@zlatanvasovic
Copy link
Contributor Author

Yes, makes sense. I already excluded it.

WOW: build is failling.

@mdo mdo modified the milestones: v3.2.0, v3.1.1 Feb 13, 2014
@mdo
Copy link
Member

mdo commented Feb 13, 2014

This might have to wait until v3.2. If we can get all the kinks (for these changes) worked out by 8am Pacific time this morning, we can merge it. Otherwise, punting to the next release.

@zlatanvasovic
Copy link
Contributor Author

It's best to leave this for next release. Do the others agree?

2014-02-13 10:11 GMT+01:00 Mark Otto [email protected]:

This might have to wait until v3.2. If we can get all the kinks (for these
changes) worked out by 8am Pacific time this morning, we can merge it.
Otherwise, punting to the next release.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666#issuecomment-34960028
.

Zlatan Vasović - ZDroid

@mdo mdo modified the milestones: v3.2.0, v3.1.1 Feb 13, 2014
@XhmikosR
Copy link
Member

Totally. But we should have the customizer warning for Safari and IE 8-9 if possible.

@zlatanvasovic
Copy link
Contributor Author

Yes.

2014-02-13 10:28 GMT+01:00 XhmikosR [email protected]:

Totally. But we should have the customizer warning for Safari and IE 8-9
if possible.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666#issuecomment-34961164
.

Zlatan Vasović - ZDroid

@ssorallen
Copy link
Contributor

@cvrebert I haven't found a synchronous blob: URL support check yet, but setting an image's src and waiting for onload or onerror correctly detects Safari 6, 6.1, and 7: https://github.com/ssorallen/blob-feature-check

@zlatanvasovic
Copy link
Contributor Author

Thanks Ross, it looks good to me. Could we use it to check for Safari bugs?

2014-02-13 19:28 GMT+01:00 Ross Allen [email protected]:

@cvrebert https://github.com/cvrebert I haven't found a synchronous
blob: URL support check yet, but setting an image's src and waiting for
onload or onerror correctly detects Safari 6, 6.1, and 7:
https://github.com/ssorallen/blob-feature-check


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666#issuecomment-35008896
.

Zlatan Vasović - ZDroid

@ssorallen
Copy link
Contributor

@zdroid Definitely, most things I post on GitHub are public domain. It's not as straightforward as a synchronous test, but I imagine most uses of the customize page will include visits long enough for a couple-millisecond check to occur.

@zlatanvasovic
Copy link
Contributor Author

Yes, right.

2014-02-14 6:18 GMT+01:00 Ross Allen [email protected]:

@zdroid https://github.com/ZDroid Definitely, most things I post on
GitHub are public domain. It's not as straightforward as a synchronous
test, but I imagine most uses of the customize page will include visits
long enough for a couple-millisecond check to occur.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12666#issuecomment-35056746
.

Zlatan Vasović - ZDroid

@cvrebert
Copy link
Collaborator

I'm thinking that #12773 supersedes this.

@zlatanvasovic
Copy link
Contributor Author

Yes, it looks fine.

@XhmikosR
Copy link
Member

I will try 12773 when I'm back and comment there.

@zlatanvasovic zlatanvasovic deleted the customizer-warning branch March 4, 2014 09:40
@mdo mdo mentioned this pull request Mar 7, 2014
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.

Customizer: Show a warning when IE <=9 or Safari is used
6 participants