Found the commit that fixes this https://gerrit-review.googlesource.com/c/gerrit/+/393975. This was incorrectly reverted in https://gerrit-review.googlesource.com/c/gerrit/+/395140 by mistake. I've re-done it with https://gerrit-review.googlesource.com/c/gerrit/+/429837.

In T358762#9888333, @hashar wrote:

Fun thing I have noticed is the <a> element shallows the closing bracket and the href contains the closing bracket. So it is more complicated than just a ; being added. I think the link is generated by:

polygerrit-ui/app/utils/link-util.ts

function createLinkTemplate(
  href: string,
  displayText: string,
  prefix?: string,
  suffix?: string
) {
  return `${
    prefix ?? ''
  }<a href="${href}" rel="noopener noreferrer" target="_blank">${displayText}</a>${
    suffix ?? ''
  }`;
}

Which would be fed:

prefix	<
href	https://example.com>
displayText	https://example.com>
suffix	;

The method is apparently called solely for processing commentlink which are regex feeding the above function. Maybe one of our configured comment link is the cause of the issue?

Looks like the repo is now https://github.com/googlefonts/roboto-classic and not https://github.com/google/roboto

Did you clear your browser cache? it works for me.

In T362269#9873138, @Aklapper wrote:

In T362269#9714170, @Paladox wrote:

Does this work for you now?

@Paladox: Has something related changed server-side in the meantime?

In T344891#9852140, @Aklapper wrote:

In T344891#9811339, @Paladox wrote:

This will be fixed with T365328.

On some cheap Android phone it initially looks like this:

It was not super obvious that I have to drag the viewport sideways to get this:

Yeh, it’s a hack. I’ve fixed it on the master branch of Gerrit by creating a mobile view. Please visit https://gerrit-review.googlesource.com/q/status:open+-is:wip to see what it’s like and do give feedback!

Added here: https://gerrit-review.googlesource.com/c/gerrit/+/426018

Oh... I have the permissions to create one. Have done so here: https://github.com/highlightjs/highlightjs-epp

I'd ask them to create a hjs repo for you similar to highlightjs-closure-templates so that we can include it in gerrit.

Does this issue still happen to you?

This will be fixed with T365328.

In T364484#9781195, @thcipriani wrote:

I note the review for this change mentioned making the message longer: https://gerrit.wikimedia.org/r/c/operations/puppet/+/1027726

an example I found https://github.com/GerritCodeReview/plugins_checks-jenkins/blob/master/web/fetcher.ts#L139

I think that we would *use* https://github.com/GerritCodeReview/gerrit/blob/4f82b6d02ac5517e8db3771b17fa28da0a0b9677/polygerrit-ui/app/models/checks/checks-model.ts#L158 in https://gerrit.wikimedia.org/r/plugins/gitiles/operations/software/gerrit/+/refs/heads/deploy/wmf/stable-3.8/plugins/wm-checks-api.js#631.

In T214631#9746166, @thcipriani wrote:

Guessing this no longer works since gerrit moved to lit, is that right @Paladox ?

(also, seems we've undeployed the wikimedia gerrit plugin judging from our latest release)

I do wonder how trivial this would be to implement as a new js plugin? This is something that someone mentioned in the latest developer satisfaction survey comments.

I’ve disabled it. Thanks.

Is this still an issue?

Is this still an issue (also a + is that gerrit supports rebasing chained changes now via the UI).

Is this still an issue?

(This was discovered by someone else)

The above shows then end “. It’s just not obvious as it has a similar colour to the tabs when zoomed out but is blue zoomed in.

This appears to be because items are being pushed out of screen. I did a hack https://gerrit-review.googlesource.com/c/gerrit/+/420278. It's an ugly hack but it does the job. The better fix and something I'm pushing and looking at is to create a mobile optimised header that pushes these drop downs to a side bar (or aka hamburger).

I think this may be fixed in gerrit 3.9 based on testing locally. I can't reproduce on master but I remember work on the diff screen being done for that release.

Oh... It misses the end " bit.

This seems fixed at least scroll bars show for me now.

You can search state:active or state:read-only in the UI. Doesn't seem to support hidden.

In T343471#9722290, @Jdforrester-WMF wrote:

In T343471#9722276, @Paladox wrote:

Does this still not work (gerrit is now on 3.8)?

AIUI the web editor in gerrit is broken since we moved to 3.8, so I don't think we can test yet.

Does this still not work (gerrit is now on 3.8)?

Does this work for you now?

I think you need to clear your browser cache. It works for me and Polymer isn't use any more for the codemirror gerrit plugin.

Closing as resolved. I'm not sure what the process is for opening the task to public? Is it once all those changes were merged that it can?

I've merged it and back ported it to all the stable releases up to 1.39 (so 1.41 & 1.40)

Thank you for reporting! Would you mind doing the patch on Gerrit and I’ll +2 it as soon as you’ve done it please?

From what I remember it's https://github.com/GerritCodeReview/gerrit/blob/stable-3.7/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts#L629. I last took a dive years ago so I'm a little fuzzy. But I do remember it was within there.

This is fixed in gerrit 3.9. This is to do with the migration to lit that to get it working needed to use the change model. The changes couldn't be back-ported because they are big and would require a lot being backported.

I believe gerrit 3.8 may fix the problem. It got rid of a library that had problems. Although I can't guarantee as I've only tested on master.

This is a upstream issue but testing on master it appears it's fixed.

I don't have any reproduction steps :/

In T355747#9515652, @thcipriani wrote:

In T355747#9492975, @Jdlrobson wrote:

Also happening for me which is breaking my workflow - I can no longer even visually see starred changes in my code review queue.

Hrm, you only have 229 watched changes, so it should be below the 1024 limit. Are you seeing the too many items in queue thing?

I am also noticing a visual regression in my is:starred search where I don't see those starred changes as starred (and I only a few stars for testing).

I'd like to start off with a thank you for the nomination! To get into things I'd like to bring up my +2 rights on Gerrit-review that @Tgr mentions above. Whilst yes they aren't bleeding edge in the backend they are more so for the frontend. Users are more easily impacted if there was malicious code inside the frontend code. Not to mention people building or running it locally. A maintainer breaching trust would be huge and also have reputational damage especially as big companies use it such as Wikimedia and Volvo and more. I wouldn't go around merging anything I don't understand. (I show this on Gerrit-review).

Backported the fix to https://gerrit-review.googlesource.com/c/gerrit/+/405697

Seems the REST API is "indent_with_tabs" so it's true / false. So we would have to either create a new option to allow the three options above or if indent_with_tabs is true then always indent with tabs or if it's false check content for tabs otherwise use spaces.

I think a potential fix is detecting if there's tabs in the file and using that but also adding "auto" to the preference. So the preference will indicated what to do whether to check the file or force use of spaces/tabs.

In T355561#9477465, @matmarex wrote:

I guess we just need to remove -is:ignored from the queries here: https://gerrit.wikimedia.org/r/plugins/gitiles/wikimedia/+/refs/meta/dashboards/teams/platform (and in a few other places)

I can do that.

@Paladox Do you know what these queries did previously, before the feature was removed?

"ignored" needs to be removed. This was removed in gerrit 3.7.

This is fixed.

Seems this is fixed as the next button is shown for me.

Seems this is fixed. I can’t reproduce at the url described in the description nor the other one described above.

There's also:

In T335847#8862223, @Arlolra wrote:

I looked at the mediawiki-title library and it considers "/" as valid.

The log says that RESTBase is trying to reach {"internalURI"=>"http://localhost:6502/w/rest.php/cs.wiktionary.org/v3/page/pagebundle/%2F/1258994", "internalMethod"=>"get"} and getting HTTPError: 500: http_error
https://logstash.wikimedia.org/app/discover#/doc/0fade920-6712-11eb-8327-370b46f9e7a5/ecs-test-1-1.11.0-6-2023.20?id=tK2bL4gB_1-qnHEYGQy_

On scandium, executing NO_PROXY="" no_proxy="" curl -I --proxy scandium.eqiad.wmnet:80 http://cs.wiktionary.org/w/rest.php/cs.wiktionary.org/v3/page/pagebundle/%2F/1258994 returns HTTP/1.1 500 Internal Server Error

Without the -I it's an empty response. Locally, though, I'm able to get a response from that page.

The log for the 500 error says PHP Fatal Error: Allowed memory size of 1468006400 bytes exhausted (tried to allocate 53248 bytes)
https://logstash.wikimedia.org/app/discover#/doc/logstash-*/logstash-default-1-7.0.0-1-2023.05.18?id=BquqL4gBs53OSt3dH-Ic

So, it must be something about the content of that page that makes Parsoid quickly consume memory. Let's see.

It was our http proxy.

Commenting out this https://github.com/miraheze/mediawiki/blob/REL1_40/includes/http/GuzzleHttpRequest.php#L121 gets it working.

In T351900#9355822, @Paladox wrote:

> $test = new GuzzleHttpRequest( 'http://swift-lb.miraheze.org/sonicpediawiki/thumb/8/85/SX57_Knuckles_and_Cosmo.jpeg/800px-SX57_Knuckles_and_Cosmo.jpeg', [ 'method' => 'HEAD', 'timeout' => 1 ] );

> var_dump($test->getStatus());
int(200)

I forgot ->execute()

var_dump($response);

object(GuzzleHttp\Psr7\Response)#559 (6) {

["reasonPhrase":"GuzzleHttp\Psr7\Response":private]=>
string(2) "OK"
["statusCode":"GuzzleHttp\Psr7\Response":private]=>
int(200)
["headers":"GuzzleHttp\Psr7\Response":private]=>
array(12) {
  ["Content-Type"]=>
  array(1) {
    [0]=>
    string(10) "image/jpeg"
  }
  ["Content-Length"]=>
  array(1) {
    [0]=>
    string(5) "75995"
  }
  ["X-Object-Meta-Sha1Base36"]=>
  array(1) {
    [0]=>
    string(31) "09twibbekbm6a71x006eehab1hejxsj"
  }
  ["Content-Disposition"]=>
  array(1) {
    [0]=>
    string(57) "inline;filename*=UTF-8''SX57_Knuckles_and_Cosmo.jpeg.jpeg"
  }
  ["Etag"]=>
  array(1) {
    [0]=>
    string(32) "11a744edf44e10792d5bf56bd16c4b75"
  }
  ["Last-Modified"]=>
  array(1) {
    [0]=>
    string(29) "Sun, 05 Nov 2023 13:26:26 GMT"
  }
  ["X-Timestamp"]=>
  array(1) {
    [0]=>
    string(16) "1699190785.38844"
  }
  ["Accept-Ranges"]=>
  array(1) {
    [0]=>
    string(5) "bytes"
  }
  ["Access-Control-Allow-Origin"]=>
  array(1) {
    [0]=>
    string(1) "*"
  }
  ["X-Trans-Id"]=>
  array(1) {
    [0]=>
    string(34) "tx2b281b55765c4e90b2050-00655f92ce"
  }
  ["X-Openstack-Request-Id"]=>
  array(1) {
    [0]=>
    string(34) "tx2b281b55765c4e90b2050-00655f92ce"
  }
  ["Date"]=>
  array(1) {
    [0]=>
    string(29) "Thu, 23 Nov 2023 17:58:38 GMT"
  }
}
["headerNames":"GuzzleHttp\Psr7\Response":private]=>
array(12) {
  ["content-type"]=>
  string(12) "Content-Type"
  ["content-length"]=>
  string(14) "Content-Length"
  ["x-object-meta-sha1base36"]=>
  string(24) "X-Object-Meta-Sha1Base36"
  ["content-disposition"]=>
  string(19) "Content-Disposition"
  ["etag"]=>
  string(4) "Etag"
  ["last-modified"]=>
  string(13) "Last-Modified"
  ["x-timestamp"]=>
  string(11) "X-Timestamp"
  ["accept-ranges"]=>
  string(13) "Accept-Ranges"
  ["access-control-allow-origin"]=>
  string(27) "Access-Control-Allow-Origin"
  ["x-trans-id"]=>
  string(10) "X-Trans-Id"
  ["x-openstack-request-id"]=>
  string(22) "X-Openstack-Request-Id"
  ["date"]=>
  string(4) "Date"
}
["protocol":"GuzzleHttp\Psr7\Response":private]=>
string(3) "1.1"
["stream":"GuzzleHttp\Psr7\Response":private]=>
object(GuzzleHttp\Psr7\Stream)#556 (7) {
  ["stream":"GuzzleHttp\Psr7\Stream":private]=>
  resource(1229) of type (stream)
  ["size":"GuzzleHttp\Psr7\Stream":private]=>
  NULL
  ["seekable":"GuzzleHttp\Psr7\Stream":private]=>
  bool(true)
  ["readable":"GuzzleHttp\Psr7\Stream":private]=>
  bool(true)
  ["writable":"GuzzleHttp\Psr7\Stream":private]=>
  bool(true)
  ["uri":"GuzzleHttp\Psr7\Stream":private]=>
  string(10) "php://temp"
  ["customMetadata":"GuzzleHttp\Psr7\Stream":private]=>
  array(0) {
  }
}

}

for instance

I found a fix: https://github.com/miraheze/ManageWiki/commit/4fb5fd1a365b55889b632d11ed07239f2582c1bf. Not sure what changed in php 8.2 to require that. I saw on production:

Looks like this is caused by https://github.com/wikimedia/mediawiki/commit/2b0b187bae8a42b4dca68a446ba5050bb7de6c50. Default value if not set is a null per getDefault(). Using 'default' => 'File' seemed to fix it but feel like it's a hack.

We could revert https://github.com/wikimedia/mediawiki/commit/4727ed1a9ccb6c321e536f0249dafc1f4d160d88 until a fix is found?

In T344189#9283792, @Paladox wrote:

Found it. It's the type in loadInputFromParameters.

Switch from:

	public static function loadInputFromParameters( $fieldname, $descriptor,
		HTMLForm $parent = null
	)

to

	public static function loadInputFromParameters( $fieldname, $descriptor, $parent = null)

seems to fix it.

Found it. It's the type in loadInputFromParameters.

I can reproduce it on Special:Preferences.

... And further I get to $descriptor['parent'] = $parent; (which the param passed is $this.

Even further, I got to $setSection[$fieldname] = $field;.

In T344189#9279380, @Func wrote:

HTMLForm has been widely used in the core and many extensions, if the bug is only reproducible with interfaces created by the ManageWiki extension, there must be some unusual pattern in that extension.

Digging, I can reproduce the OOM if I just do: error_log(print_r($loadedDescriptor, true));.