Page MenuHomePhabricator

gerritbot comment URLs replace + with space
Closed, ResolvedPublic

Description

Random example:

Change 608433 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Migrate Client MovePageNotice hook to extension JSON

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/ /608433

That URL should be https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/608433, not https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/ /608433 (though the “space” URL also seems to work – Gerrit redirects it to the “plus” URL). This seems to have changed with the Gerrit 3.2 upgrade.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 30 2020, 9:55 AM
QChris claimed this task.EditedJun 30 2020, 10:34 PM
QChris added a subscriber: QChris.

We noticed that during testing the build. But as you said the URLs work. Even if you copy/paste the link text and use it as URL, it works.

But we can work around the space vs. + to get a better URL, if it's annoying to see.
Actually ... since you filed the bug, it annoys you. I'll see to getting that changed :-)

I seem to remember a case where it didn’t work, but I’m not sure where it was.

Maybe when wikibugs copied those comments into IRC?

Tgr added a subscriber: Tgr.Jul 1 2020, 2:53 PM

+1 that this looks broken and is thus somewhat confusing. + and space are kinda equivalent in the query part of the URL, but not in the path.
Also will probably break autolinking in email clients etc.

Change 608953 had a related patch set uploaded (by QChris; owner: Christian Aistleitner):
[operations/puppet@production] gerrit: Make implicit its templates explicit

https://gerrit.wikimedia.org/r/c/operations/puppet/ /608953

Change 608954 had a related patch set uploaded (by QChris; owner: Christian Aistleitner):
[operations/puppet@production] gerrit: Format short gerrit URLs in phabricator comments

https://gerrit.wikimedia.org/r/c/operations/puppet/ /608954

Change 608953 merged by Dzahn:
[operations/puppet@production] gerrit: Make implicit its templates explicit

https://gerrit.wikimedia.org/r/c/operations/puppet/ /608953

Change 608954 merged by Dzahn:
[operations/puppet@production] gerrit: Format short gerrit URLs in phabricator comments

https://gerrit.wikimedia.org/r/c/operations/puppet/ /608954

QChris closed this task as Resolved.Jul 2 2020, 8:59 PM

The fix has been deployed. (See the last few entries in T775)