element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
GNU Affero General Public License v3.0
11.23k stars 2k forks source link

Disabled cookies gives misleading "Browser can't run element" error #14533

Open 0cjs opened 4 years ago

0cjs commented 4 years ago

Description

Loading app.element.io in a browser with cookies disabled gives a misleading error message about the browser not being able to run Element, when in reality the issue may be fixed simply by enabling cookies.

(I am not familiar with the frequency with which people browse with cookies disabled, but doing so is a minimally invasive process, at least with Chrome where the interface allows you to persistently enable cookies for a site with three quick mouse clicks within an existing tab.)

Steps to reproduce

Version information

t3chguy commented 4 years ago

when in reality the issue may be fixed simply by enabling cookies.

There is no way to tell from a code perspective whether your browser had the capability of local storage ripped our or merely disabled.

Cookies actually aren't needed, but most "Cookie disabling" methods actually prevent IndexedDB or Local Storage from working, those are necessary for a client-side app to function. That's where your credentials and keys get stored.

0cjs commented 4 years ago

Still, wouldn't explaining a bit more about what the code had tried to do and failed improve the error message? Right now in this situation it's rather misleading, especially since the suggested solution ("install Chrome") is clearly incorrect in this case.

t3chguy commented 4 years ago

Design clearly outlined to not specify the internals of the error but will pass it back to them again

0cjs commented 4 years ago

I see. Could you point me at wherever "they" documented the rationale for that design decision? Perhaps I'm missing something here.

t3chguy commented 4 years ago

They didn't want to have to maintain the vast number of errors that are possible at this time

0cjs commented 4 years ago

And those were the only two approaches they considered? Maintain a "vast" number of error messages or give an incorrect error message in some circumstances?

Anyway, seems like you guys aren't interested in addressing this, so I'll just remember that and we can move on.

t3chguy commented 4 years ago

I raised it for design to review in case their opinion has changed since already.

0cjs commented 4 years ago

Ok, thanks!

niquewoodhouse commented 4 years ago

Feels to me, just from reading this, like even just saying underneath the browser links "If you have these installed and up to date already and still can't see Element, check if your cookies are disabled. Element requires cookies to run" might be of a lot of help, and doesn't require updating a big list of things?

Or am I missing the point a bit?

t3chguy commented 4 years ago

still can't see Element, check if your cookies are disabled. Element requires cookies to run

It doesn't require Cookies though but some browsers upon disabling "Cookies" disable a wide assortment of features.

This view comes up when any of ~30 tested browser features are missing so that would be even more misleading IMO

niquewoodhouse commented 4 years ago

Ok, thanks for feeding back, so is it a case of my suggested copy being clearer?

If you have these installed and up to date already and still can't see Element, check if your cookies are disabled as it might be that your browser has made some features of Element reliant on cookies being enabled."

t3chguy commented 4 years ago

Sure, it can also happen for when your computer is running low on Storage or due to various extensions

the actual missing feature is either Local Storage or IndexedDB, saying we use cookies might upset privacy folk

0cjs commented 4 years ago

Yeah, I can see that there's probably a difference between using Local Storage and locally storing cookies that someone who knows the details about these things would distinguish. But I think it's possible to come up with a message that's reasonably clear about what's going on to those who enable and disable these sorts of things without going into massive detail about it. Perhaps something along the lines of:

The Element App is unable to store the local data it needs to run.

You may be using a browser that doesn't support the features needed for this, or these features may be disabled by an extension or a setting such as "Disable cookies."

We suggest you use Chrome, Firefox or Safari and ensure that your browser and extension settings do not disable local storage features.

Note that I'm using the generic term "local storage" here, rather than mentioning any one particular technology. There might be a better way of phrasing that to make it clear that one's talking about an entire set of related features rather than one particular feature.

t3chguy commented 4 years ago

This'll need to go through @nadonomy as he explicitly stated we don't want to show error details when I asked him during building this

0cjs commented 4 years ago

It would be good to know the scenarios he was envisioning where it was better not to give error details. If it was along the lines of, "We don't want to show a message that says that the somebrowser.storage.rfc9999.frob call failed with return code 0x173," that's perfectly understandable. But the current situation goes quite a ways beyond just avoiding being overly specific in that way, and I'm not clear on how the current message is supposed to make it easier, rather than harder, to provide technical support and resolve problems.

t3chguy commented 4 years ago

The various possible states are listed here: https://github.com/vector-im/riot-web/blob/57d2026a4007a3de8aec930988a13515598d3d7d/.modernizr.json#L8-L36

and here:

https://github.com/vector-im/riot-web/blob/9bb1f99bd9bc7155d5277a529962299007e4a245/src/vector/index.ts#L52-L59

Also what if there are multiple errors, just saying oh its because Cookies will then let the user resolve that issue only to be met with others which may or may not have useful descriptions