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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NcReferenceList): Resolve relative URLs before fetching references #5272

Merged
merged 1 commit into from Feb 20, 2024

Conversation

mejo-
Copy link
Contributor

@mejo- mejo- commented Feb 20, 2024

Resolve relative URLs and those without a location using window.location before fetching references for them. This fixes e.g. getting references for relative links to other pages in Collectives.

馃弫 Checklist

  • 鉀戯笍 Tests are included or are not applicable
  • 馃摌 Component documentation has been extended, updated or is not applicable
  • 3锔忊儯 Backport to next requested with a Vue 3 upgrade

Resolve relative URLs and those without a location using
`window.location` before fetching references for them. This fixes e.g.
getting references for relative links to other pages in Collectives.

Signed-off-by: Jonas <jonas@freesources.org>
@mejo- mejo- added bug Something isn't working 3. to review Waiting for reviews feature: richtext Related to the richtext component labels Feb 20, 2024
@susnux susnux added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 20, 2024
@susnux susnux added this to the 8.7.1 milestone Feb 20, 2024
@mejo- mejo- merged commit 71740f8 into master Feb 20, 2024
18 checks passed
@mejo- mejo- deleted the fix/reference_full_url branch February 20, 2024 20:29
@mejo-
Copy link
Contributor Author

mejo- commented Feb 20, 2024

/backport to next

@Antreesy
Copy link
Contributor

Breaking messages in Talk, if couldn't resolve a URL:

Loading for all messages Failing at more-than-just-link messages
image image

@mejo-
Copy link
Contributor Author

mejo- commented Feb 22, 2024

@Antreesy what do you mean with "if couldn't resolve a URL"? The links in your screenshot already look like full URLs, so they shouldn't get altered by the changes from this PR. Or are links to other calls inserted as absolute links without a location part in Talk?

@Antreesy
Copy link
Contributor

Antreesy commented Feb 22, 2024

The links in your screenshot already look like full URLs, so they shouldn't get altered by the changes from this PR.

They're not, but new URL creates an object for every text, whether it has a link or not.
BEFORE change, there was no attempt to resolve a reference, and messages don't have the spinner
Now, if messages which includes both plain text and link, references couldn't be loaded (third example)

I'm looking into how to fix that

@mejo-
Copy link
Contributor Author

mejo- commented Feb 22, 2024

Ah, so the problem is that that Talk passes text that contains more than just an URL to NcReferenceList. In Text, we check first if the passed text is merely an URL and only pass it in this case.

mejo- added a commit to nextcloud/text that referenced this pull request Feb 24, 2024
See discussions here for further information:
* nextcloud-libraries/nextcloud-vue#5290
* nextcloud-libraries/nextcloud-vue#5272

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/text that referenced this pull request Feb 24, 2024
See discussions here for further information:
* nextcloud-libraries/nextcloud-vue#5290
* nextcloud-libraries/nextcloud-vue#5272

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/text that referenced this pull request Feb 24, 2024
See discussions here for further information:
* nextcloud-libraries/nextcloud-vue#5290
* nextcloud-libraries/nextcloud-vue#5272

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/text that referenced this pull request Feb 26, 2024
See discussions here for further information:
* nextcloud-libraries/nextcloud-vue#5290
* nextcloud-libraries/nextcloud-vue#5272

Signed-off-by: Jonas <jonas@freesources.org>
mejo- added a commit to nextcloud/text that referenced this pull request Feb 26, 2024
See discussions here for further information:
* nextcloud-libraries/nextcloud-vue#5290
* nextcloud-libraries/nextcloud-vue#5272

Signed-off-by: Jonas <jonas@freesources.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug Something isn't working feature: richtext Related to the richtext component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants