Skip to content

Conversation

@zzet
Copy link
Contributor

@zzet zzet commented Aug 25, 2014

The code checks whether the user's browser to the list of supported. Are listed on the page: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/requirements.md#supported-webbrowsers. If the browser is outdated or unsupported - displays information block.

All rules are described in config file: config/initializers/browser_modern_rules.rb

@TeatroIO
Copy link

I've prepared a stage. Click to open.

@Razer6
Copy link
Member

Razer6 commented Aug 25, 2014

@zzet Is there a possibility to hide that flash?

@zzet
Copy link
Contributor Author

zzet commented Aug 25, 2014

@Razer6 At this moment - no. I can add close button, or auto-hide (better imho) if it need.

@dblessing
Copy link
Member

Have there been issues with people using unsupported browsers and having issues? If there haven't been some strong cases I'm really not in favor of this. Lots of browsers other than the "officially" supported versions work for a lot of sites/apps. It drives me nuts when sites display this message to me but things work fine. Also, this is another list that developers have to keep up.

@zzet
Copy link
Contributor Author

zzet commented Sep 9, 2014

@dblessing Scratch where it itches. I periodically enters feedback from users who use browsers from the list, the fact that something is not working or is not working properly. I'm tired of wasting time on debriefing. Of course, it is possible to reduce the requirements for browsers, and to expand the list. And also make the message concealed.

@dblessing
Copy link
Member

I'm going to give this a 👎 @randx do you have an opinion on this?

@dzaporozhets
Copy link
Contributor

I am against white-listing browsers. GitLab can work fine in some unknown browsers based on webkit or so. Also it can works perfectly on older Chrome. I think this message only makes sense in cases like IE6-7 where its hardly usable. In this case blacklisting makes more sense.

@zzet
Copy link
Contributor Author

zzet commented Sep 10, 2014

@randx sorry, but I'm hate blacklist-way. We have some requirements and this PR based on this doc. I'm added only minimum in browser list (we can add some another). I prefer more friendly rules, but if something not tested and can work wrong - mb better to say about it?

What about old versions - I had some problems with old Firefox, Chrome, Chromium... After run this feature in production - users updated they browsers and no problem now.

Of course - it is low-low-low priority feature :) And I'm not sure what you need it. We create it in our fork and send feature in main repo.

Thx )

@dblessing
Copy link
Member

@zzet Thanks for your contribution. I'm closing this PR with it's current whitelist implementation. We might accept something for blacklisting IE6-7 browsers as Dmitriy noted.

@dblessing dblessing closed this Oct 9, 2014
@vicvega
Copy link
Contributor

vicvega commented Nov 18, 2014

@zzet, I'm considering submitting a PR based on your own code, but implementing a blacklist, as requested
is it fine for you?

@zzet
Copy link
Contributor Author

zzet commented Nov 18, 2014

@vicvega no problem :)

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

Development

Successfully merging this pull request may close these issues.

6 participants