Skip to content

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Oct 26, 2023

Description:

Fixes #21446

Corrects an invalid URL causing a JS crash when an update is detected.

Review

@bx80 bx80 added Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. labels Oct 26, 2023
@bx80 bx80 added this to the 5.0.0 milestone Oct 26, 2023
@bx80 bx80 self-assigned this Oct 26, 2023
@michalkleiner
Copy link
Contributor

Should we perhaps wrap https://github.com/matomo-org/matomo/blob/5.x-dev/plugins/Morpheus/javascripts/piwikHelper.js#L36-L58 in try {} catch(err) {} block and return empty string if there's an error thrown? It would prevent the hard fail, but at the same time we would not have caught the incorrect input string 🤷.

@bx80
Copy link
Contributor Author

bx80 commented Oct 26, 2023

I think that would be a better outcome than a hard fail, it's possible that future usage of the external link methods, either in core or plugins, could result in invalid URLs causing a similar crash, so minimizing the potential impact makes sense 👍

@bx80 bx80 requested review from michalkleiner and caddoo October 29, 2023 21:56
@bx80 bx80 merged commit c5bce87 into 5.x-dev Oct 30, 2023
@bx80 bx80 deleted the m21446-fix-url-error-on-update branch October 30, 2023 02:09
caddoo pushed a commit that referenced this pull request Nov 5, 2023
* Fix incorrect URL parameter

* Fix incorrect URL parameter in JS tracking code generator component

* built vue files

* Improve error handling of invalid URLs in _pk_externalRawLink

* Update plugins/Morpheus/javascripts/piwikHelper.js

Co-authored-by: Michal Kleiner <michal@innocraft.com>

* built vue files

* Bump stalled tests

---------

Co-authored-by: Michal Kleiner <michal@innocraft.com>
Co-authored-by: michalkleiner <michalkleiner@users.noreply.github.com>
Co-authored-by: bx80 <bx80@users.noreply.github.com>
caddoo pushed a commit that referenced this pull request Nov 5, 2023
* Fix incorrect URL parameter

* Fix incorrect URL parameter in JS tracking code generator component

* built vue files

* Improve error handling of invalid URLs in _pk_externalRawLink

* Update plugins/Morpheus/javascripts/piwikHelper.js

Co-authored-by: Michal Kleiner <michal@innocraft.com>

* built vue files

* Bump stalled tests

---------

Co-authored-by: Michal Kleiner <michal@innocraft.com>
Co-authored-by: michalkleiner <michalkleiner@users.noreply.github.com>
Co-authored-by: bx80 <bx80@users.noreply.github.com>
caddoo pushed a commit that referenced this pull request Nov 5, 2023
* Fix incorrect URL parameter

* Fix incorrect URL parameter in JS tracking code generator component

* built vue files

* Improve error handling of invalid URLs in _pk_externalRawLink

* Update plugins/Morpheus/javascripts/piwikHelper.js

Co-authored-by: Michal Kleiner <michal@innocraft.com>

* built vue files

* Bump stalled tests

---------

Co-authored-by: Michal Kleiner <michal@innocraft.com>
Co-authored-by: michalkleiner <michalkleiner@users.noreply.github.com>
Co-authored-by: bx80 <bx80@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Development

Successfully merging this pull request may close these issues.

[Bug]: Blank screen crash when showing update notification
2 participants