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

[stable28] Check link protocol #6491

Open
wants to merge 3 commits into
base: stable28
Choose a base branch
from
Open

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Oct 2, 2024

Backport of #6486

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Remove all the empty commits

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@max-nextcloud
Copy link
Collaborator

This backport is more involved as we did not have the link bubble yet in 28.

@max-nextcloud
Copy link
Collaborator

I think the backport is fine now - I'm still unsure though if this is really the behavior we want.
With this change one can only get at the urls from links with extraordinary protocols by editing the link. Clicking the link itself won't open it and right click won't work.

Probably not so bad anyway though as 28 is pretty old and from 29 on we have the link bubble.

@juliusknorr juliusknorr marked this pull request as ready for review November 18, 2024 07:10
@juliusknorr juliusknorr added the bug Something isn't working label Nov 18, 2024
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud
Copy link
Collaborator

Cypress component tests seem to fail fairly reliable.

@juliusknorr
Copy link
Member

Maybe they did break with #6501 ? Before that stable28 seemed fine https://github.com/nextcloud/text/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Amerged+base%3Astable28

@max-nextcloud
Copy link
Collaborator

Maybe they did break with #6501 ? Before that stable28 seemed fine https://github.com/nextcloud/text/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Amerged+base%3Astable28

Good catch. @nextcloud/webpack-vue-config bump also bumped webpack-dev-server.

@max-nextcloud
Copy link
Collaborator

Okay... so i see several options how to handle this:

  1. downgrade @nextcloud/webpack-vue-config so webpack-dev-server can be downgraded.
  2. upgrade cypress and skip tests that crash it.
  3. revert the move of tests from e2e to component tests.
  4. fix the actual issue with ResizeObserver loop in our code.

They all seem somewhat cumbersome. In addition this issue does not show on current versions as we are using vite there I believe.

@juliusknorr
Copy link
Member

I'd go with 1 for the stable branch for now to get this pr in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
Status: 🏗️ In progress
Development

Successfully merging this pull request may close these issues.

4 participants