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

App does not always work on IE #21

Closed
3 tasks done
oparoz opened this issue Feb 12, 2015 · 31 comments · Fixed by owncloud/core#8072
Closed
3 tasks done

App does not always work on IE #21

oparoz opened this issue Feb 12, 2015 · 31 comments · Fixed by owncloud/core#8072
Assignees

Comments

@oparoz
Copy link
Contributor

oparoz commented Feb 12, 2015

There are several issues when loading the app in Internet Explorer

Next steps

  • Warn users (and be nice)
  • Use a supported polyfill which can handle multiple EventSource objects
  • Fix the image downloading
@ghost
Copy link

ghost commented Feb 13, 2015

There is no errors in "F12" for IE11, just the spinning wheel -> maybe iframe/urlength related?

@oparoz
Copy link
Contributor Author

oparoz commented Feb 14, 2015

@f-s - Maybe... We won't know for sure until that part is fixed :|
@libasys has found out that there are some issues with some of the Javascript, so I'll investigate that later.
libasys/ownCloud-7.0.3@08fe372

@oparoz
Copy link
Contributor Author

oparoz commented Mar 12, 2015

So, @deMattin, Could you just let me know if you see at least one request for thumbnails in the network tab of the debugger?
It's been reported that the EventSource hack put in place in core doesn't work too well, but I don't know yet if that's the issue.

@oparoz
Copy link
Contributor Author

oparoz commented Mar 13, 2015

Yep, the app gets the data, but IE can't display it.
EDIT: Confirmed as not working with Gallery

@oparoz
Copy link
Contributor Author

oparoz commented Mar 14, 2015

It might be possible to support IE11 at some point, but in the meantime, we're doing #92

@oparoz oparoz changed the title App does no load thumbnails on IE11 App does no work on IE11 Mar 15, 2015
@oparoz
Copy link
Contributor Author

oparoz commented Mar 15, 2015

Good news, full screen previews work again. it was my mistake 😳

@oparoz oparoz self-assigned this Mar 15, 2015
@oparoz
Copy link
Contributor Author

oparoz commented Mar 15, 2015

Fixed in 4efcbd8

The only problem left is with thumbnails. There is a hack in place to make it work, but I'm waiting on a more sturdy solution.

@deMattin
Copy link

Hmm - for me with IE now thumbnails work and preview not!?

Could it be an temporary solution to enhance the preview message, that error generating preview is not possible only with IE, yet. So you know, that there is a problem caused by usage of IE and not a general one as the mesage intents now.
You gave this sort of message in gallery (thumbnail) view - but this works well with IE11

And "use a modern browser like chrome or firefox" is a bit embarrassing.
IE11 IS a modern (and actual) browser - even if you or me don't like or use it.

@oparoz
Copy link
Contributor Author

oparoz commented Mar 16, 2015

You need to delete the content of the asset folder and use 2.0.7 to be able to see the IE improvements.

@deMattin
Copy link

Ok - that has to wait until I get OC 8.0.2.

But I don't understand, that you said, the thumbnails don't work.
The yesterdays master branch (last 2.0.6?) did work well with thumbnails on IE11 but not with full preview,

Why does improved 2.0.7 (with now showing full previews) not show thumbnails any more if this worked with 2.0.6?
Or did you implement there the "hack" for testing?

BTW: the actual performance-fixes branch doesn't work with IE11!

@oparoz
Copy link
Contributor Author

oparoz commented Mar 16, 2015

The hack is in 2.0.6 already. There is a short timeout to make sure we get all the thumbnails. Without it, the polyfill just hangs. So this may not work well for everybody and it's slowing things down a little. Still better than nothing :D

@oparoz
Copy link
Contributor Author

oparoz commented Mar 17, 2015

I'm still not happy about the notification for IE11 users. It pollutes the UI for users who may not be able to do anything about it.

Does anybody know if there is a way to identify logged-in admins?

@deMattin
Copy link

Does this in core/user.php help?

/**
     * Check if the user is an admin user
     *
     * @param string $uid uid of the admin
     * @return bool
     */
    public static function isAdminUser($uid) {
        if (OC_Group::inGroup($uid, 'admin') && self::$incognitoMode === false) {
            return true;
        }
        return false;
    }

/**
     * get the user id of the user currently logged in.
     *
     * @return string uid or false
     */
    public static function getUser() {
        $uid = \OC::$server->getSession() ? \OC::$server->getSession()->get('user_id') : null;
        if (!is_null($uid) && self::$incognitoMode === false) {
            return $uid;
        } else {
            return false;
        }
    }

What are you up to?

@oparoz
Copy link
Contributor Author

oparoz commented Mar 18, 2015

Now the same thing in Javascript :)

@setnes
Copy link
Contributor

setnes commented May 27, 2015

Right now I see that Gallery is blocking IE with a message to use Firefox or Chrome. I understand that the gallery thumbnail page cannot reliably be displayed using IE. Is it possible to present a visitor with a link or redirect to the files view of the folder? Users in a working gallery, can switch between the files view and the gallery view. There is not a browser block on the files view... the slideshow will even start.

As it stands, to share a link to a photos directory with someone in IE, I would need to send a link to the files using the sharing functionality in core. If I want to share a link to a photos directory with everybody else I would use a shared link from the gallery app.

In other words, I feel we should fail to core files functionality instead of preventing a user from seeing anything. I might not fully understand the limitations of IE though.

@setnes
Copy link
Contributor

setnes commented May 28, 2015

I need to clarify my last comment. IE 10 fails to load any images, so the toggle between gallery and files isn't there as a fallback. IE 11 loads images with a warning and the toggle is presented. IE 10 (and presumably below) needs some sort of fall back to the files interface. (simple link, redirect, or something else)

@oparoz
Copy link
Contributor Author

oparoz commented May 29, 2015

Thanks for testing. It's probably a problem with the polyfill we're using when multiple EventSource objects are launched simultaneously.

I agree that if it isn't working as expected, then there should be some sort of fallback.
Given the situation, I think it would be wise to show an empty Gallery, with navigation, as well as the warning.
Maybe @jancborchardt has a better idea?

Maybe we could just use one EventSource for those users... There would be no parallel processing server-side, but at least they would get something...
@setnes - What happens if you try to load a folder with just a couple of pictures on IE10? Do you get the thumbnails?

@setnes
Copy link
Contributor

setnes commented May 29, 2015

This shows up easy using the browserstack screenshot tool. I created a shared link of a gallery folder with three images in it.
https://www.browserstack.com/screenshots/

IE 8, 9, and 10 all look like this (no way to get to the file interface)...

win8_ie_10 0

IE 11 looks like this...

win8 1_ie_11 0

And for comparison, Chrome looks like this...

macmav_chrome_36 0

@oparoz
Copy link
Contributor Author

oparoz commented May 29, 2015

I'm wondering if they're blocking certain events. Will need to test in a VM. We use btoa for SVGs which isn't supported on IE < 10, but it's only in the slideshow, so I don't think it should have an impact... There might be other part of the code which doesn't work.

I've meant to register the project with one of these browser CI tools, but I haven't found the time yet.

To be honest, when looking at a solution, I didn't try to write tons of browser specific code, as I don't think browsers prior to IE11 should be supported. I try to use polyfills if they exist.
The oldest supported OS from Microsoft is Windows 7 and that runs IE11. If some people want to run older versions for some reason, then they should hire the required skills to provide patches. I think it's a total waste of resources trying to support IE8 in core or worse, it's crippling the development.

@setnes
Copy link
Contributor

setnes commented May 29, 2015

I don't think gallery needs to fully support browsers below IE11, but there does need to be a way to fall back to the file interface so the files can at least be viewed. Older versions of IE are still heavily used in the corporate world... even if everybody's home computers have long since switched to IE11. I don't have any problem with a "non supported" message for old browsers. There just needs to be a way to get to the files in those cases.

@karlitschek
Copy link

Makes sense from my point of view. But please note that if galleryplus supports less browsers then the ownCloud core then this makes shipping this as the new default gallery app a bit more difficult. This can always be an optional app of course.
Great progress by the way. I love it already.

@oparoz
Copy link
Contributor Author

oparoz commented May 29, 2015

Well, it supports one more browser than Pictures (since that doesn't work on IE at all) and that's included as default ;), but I hear you.
I think the middle ground would be for the slideshow to work on IE 9-11 and for a redirection to be put in place in the gallery view, until a proper solution can be found, if it exists. I'll know more once I can see what goes wrong in these browsers.

@karlitschek
Copy link

great idea. :-)

@oparoz oparoz changed the title App does no work on IE11 App does not work on IE11 Jun 12, 2015
@oparoz oparoz changed the title App does not work on IE11 App does no work on IE11 Jun 12, 2015
@oparoz oparoz added ready and removed On hold labels Jun 12, 2015
@oparoz oparoz changed the title App does no work on IE11 App does not always work on IE Jun 12, 2015
@oparoz
Copy link
Contributor Author

oparoz commented Jul 22, 2015

Need to add this #206 to see if it improves the situation for IE9-10.

The slideshow side also needs to be tested on those browsers. Bigshot does not work on IE8, but maybe it does on 9-10.

@ghost
Copy link

ghost commented Jul 22, 2015

@davitol @rperezb can you test this also?

@oparoz
Copy link
Contributor Author

oparoz commented Jul 22, 2015

So #206 can't work as it's designed to work with the hack put in place in core.
The polyfill we're using here works with IE10 and IE11, but a hack would also need to be used for earlier versions.

While testing I encountered various little issues with IE which prevent users from getting a smooth ride while using the slideshow.
IE is particularly bad at reporting what's wrong and just throws these errors left and right "SCRIPT16389: Unspecified error."

@rperezb
Copy link

rperezb commented Jul 23, 2015

@cmonteroluque if i am right, galleryplus app will not be released on 8.1.1 version, if so, we may check this after 8.1.1 released

@oparoz
Copy link
Contributor Author

oparoz commented Jul 23, 2015

@rperezb - The app is available for 8.0 and 8.1, but those versions are not official (as in ship with oC) and only support IE11

@rperezb
Copy link

rperezb commented Jul 24, 2015

oki doki, added this one as part of the test plans for oc 8.0.6 and 8.1.1 thx
@owncloud/qa

@oparoz
Copy link
Contributor Author

oparoz commented Jul 25, 2015

I've created a IE feature/support matrix to help avoiding wasting time testing features which don't work.
https://github.com/owncloud/galleryplus/wiki/Internet-Explorer-support

Currently, in master, the apps shows an empty gallery when IE8 is detected.

@oparoz
Copy link
Contributor Author

oparoz commented Jul 25, 2015

I'm going to close this as the app (master) generally works on IE9-11 now. New tickets can be opened for specific issues.

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

Successfully merging a pull request may close this issue.

5 participants