Skip to content

Add default_server_config fallback to element.io configs - #19695

Merged
t3chguy merged 5 commits into
element-hq:developfrom
aaronraimist:element.io-default_server_config
Jul 10, 2023
Merged

t3chguy merged 5 commits into
element-hq:developfrom
aaronraimist:element.io-default_server_config

Conversation

@aaronraimist

@aaronraimist aaronraimist commented Nov 11, 2021

Copy link
Copy Markdown
Collaborator

This doesn't solve #11154 but at least prevents it from happening on app.element.io and develop.element.io. default_server_name is also deprecated.

This PR greatly improves the user experience when matrix.org is unreachable. Currently it looks like this:

The error message doesn't provide much information about what went wrong and there is no way to switch to another server.

After this PR:

The error message actually says something meaningful and the user is able to switch to another server.

I will create a PR for desktop if this is acceptable.


This change is marked as an internal change (Task), so will not be included in the changelog.

This doesn't fix element-hq#11154 but at least prevents it from happening on app.element.io and develop.element.io. `default_server_name` is also deprecated.

I will create a PR for desktop if this is acceptable.

Signed-off-by: Aaron Raimist <aaron@raim.ist>
@aaronraimist
aaronraimist requested a review from a team as a code owner November 11, 2021 10:49
@aaronraimist

Copy link
Copy Markdown
Collaborator Author

The only downside to this is the full base_url will be displayed on the login/registration screens rather than the server name. The documentation claims default_server_config supports specifying "a server_name under the m.homeserver section to display a custom homeserver name." however it doesn't actually work.

aaronraimist/matrix-js-sdk@0303c9b would make it work however there wouldn't be any validation of the server name so it could easily be used for phishing. I'm not quite sure how you could validate it. I suppose someone hosting an instance of Element already has to be trusted not to modify Element so that may not be a big deal.

@WestXu

WestXu commented Nov 12, 2021

Copy link
Copy Markdown

Been waiting for this for a long time.

@turt2live turt2live left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! What are the chances a similar change can be made to the element-desktop repository?

@aaronraimist

Copy link
Copy Markdown
Collaborator Author

@turt2live 100%. Do you want #19695 (comment) as well?

@turt2live

Copy link
Copy Markdown
Member

oh, hmm, didn't realize that this would cause the issue mentioned in that comment. I think instead we need to spend a bit of time to think about this, sorry.

@evan0greenup

evan0greenup commented Dec 22, 2021

Copy link
Copy Markdown

@t3chguy , Do you think this is ready to be merged?

@turt2live

Copy link
Copy Markdown
Member

so in the end we've un-deprecated default_server_name, but it obviously doesn't solve the issue at hand regarding homeservers being offline. I am sort of wondering if it's something we need to fix though - it's managed to go a few years as an issue and hasn't seen much traction in terms of bug reporting.

Curious to hear thoughts on the matter.

@aaronraimist

aaronraimist commented Mar 19, 2022

Copy link
Copy Markdown
Collaborator Author

@turt2live Personally I think it is a problem. I see it pop up in #element-web:matrix.org maybe once every 2 weeks, so it certainly isn't the biggest problem but it real.

I also think it is an under reported issue because if someone gets this error, there is absolutely nothing to suggest it is a problem on your end. People likely just think: well something is broken, this sucks I'm going back to Discord/Slack/etc.

@turt2live
turt2live removed their request for review January 17, 2023 17:04
@ShadowRZ

Copy link
Copy Markdown
Contributor

Is there anything blocking this PR from being merged?

@andybalaam

Copy link
Copy Markdown
Member

We don't think we can merge this while it displays matrix-client on the front page. Would someone be willing to change the code to allow us to display matrix.org there, even when we provided this info?

@ShadowRZ

ShadowRZ commented Jun 29, 2023 via email

Copy link
Copy Markdown
Contributor

@ShadowRZ

Copy link
Copy Markdown
Contributor

My proposed patch can be found at element-web.patch.

@andybalaam

Copy link
Copy Markdown
Member

We discussed this last week and @t3chguy will work on getting this into mergeable state.

@t3chguy t3chguy added T-Task Tasks for the team like planning and removed X-Blocked labels Jul 6, 2023
@t3chguy

t3chguy commented Jul 6, 2023

Copy link
Copy Markdown
Member

So the precursor to this is #25682 once that lands this PR should be updated to not remove the default_server_name but add alongside it.

@t3chguy
t3chguy self-requested a review July 6, 2023 09:48
@t3chguy t3chguy reopened this Jul 10, 2023
@t3chguy t3chguy changed the title Switch element.io config's from using default_server_name to default_server_config Add default_server_config fallback to element.io configs Jul 10, 2023
@t3chguy

t3chguy commented Jul 10, 2023

Copy link
Copy Markdown
Member

Now that #25682 has landed this PR no longer needs to remove the preferred default_server_name

@t3chguy t3chguy removed the X-Blocked label Jul 10, 2023
@t3chguy
t3chguy merged commit 98e36ff into element-hq:develop Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants