In T345052#9182133, @dom_walden wrote:

It is curious though. Notifications for known IP subnets were being sent. Are they not done by a job as well?

In T345052#9173996, @dom_walden wrote:

I cannot get it to send a notification for a failed login on a new IP address. I can do for a known IP address. As that is the default at the moment, should that be fixed?

In T345052#9173996, @dom_walden wrote:

@tstarling With the config:

$wgLoginNotifyUseCheckUser = true;
$wgLoginNotifyUseSeenTable = false;

I cannot get it to send a notification for a failed login on a new IP address. I can do for a known IP address. As that is the default at the moment, should that be fixed?

In T346293#9177059, @Bugreporter wrote:

In the new schema it is still the case: If we hide the actor of a revision a actor_deleted row is needed, though the user itself is not hidden, so we can not know the account hidden status without ipblocks. A new column in actor_deleted like ad_user_is_hidden may help. Note the ipblocks table in some wiki may be very large due to admin bot blocking proxies.

In T346293#9165561, @Bugreporter wrote:

• MediaWiki currently allow user to redact only the performer of an edit (including log entries, etc.); a common use case is if a user accidently logged out, the user name need to be hidden. To preserve such feature we could introduce copies of existing rows on actor table, with a different deleted flag.

During request shutdown, zend_objects_store_call_destructors() calls destructors in ascending order of object ID. It doesn't attempt to follow the reference tree. So it's unsafe to do anything complex or non-local at this stage, since indeed, destructors of properties may be called before the destructor of the parent object.

ext.phonos depends on oojs-ui-widgets but it could apparently depend on oojs-ui-core instead.

I've deployed the fix on wmf.25 so the error should go away once this week's train deployment is complete.

In T194697#9126073, @daniel wrote:

when we want to provide information about the block of a given user, we just look at the most recent block log entry for that user.

Confirmed fixed.

If someone goes to the login form and enters the name of a temporary user and an incorrect password, there's no need to notify the temporary user because there is no risk of account compromise, because temporary users never have a password. Currently, a notification will be sent.

There is migratePreferences.php but it is just updating preferences that already exist in the DB. It would not be correct to skip temporary users in that situation.

In T344404#9138739, @Quiddity wrote:

I'm experiencing this bug on an external wiki, where we're trying to import the Navigation Popups gadget. Is there something we can/need to do over there to fix this? (Currently on mw 1.39.0)

I could not reproduce the actual browser limitations this is supposed to work around, just tested that the top-level autologin works. Typically 3rd-party browser limitations are only triggered by requests across different second-level domains, so I figured it will be easier to test that in production.

Logstash only shows 5 errors in the past 24 hours, so I am assuming that it is fixed rather than confirming.

The fix is actually already out. Despite the release notes tag, it went out with wmf.24.

The core documentation issue was fixed in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/954121 . The incorrect doc comment also exists in CheckUser so I will fix it there too.

User::newFromName() casts the name to a string. I changed it to use UserFactory in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/LoginNotify/+/952114 but UserFactory::newFromName() throws an exception instead of casting.

Some users will notice when this goes out with the train, so we should probably put out an explanation.

You could lsn_id make BIGINT to avoid having headaches in the future.

I believe it was @Prtksxna who expressed the concern that consecutive numbers, with most of the digits being the same between users created at a similar time, would be too hard for reviewers to distinguish in a changes list. I suggested a pseudo-random sequence as a way of making the numbers be easier to visually distinguish.

After reading https://dev.mysql.com/doc/refman/8.0/en/innodb-index-types.html I decided to add an auto-incremented primary key, instead of a time index. If you leave off the integer primary key, the secondary index needs to duplicate the 14-byte unique key, so it actually uses more space overall. Purging can be done using the primary key, if you assume it's monotonic in time.

Community Tech are looking at this, following its high ranking in the 2023 Community Wishlist Survey.

There is also T240664 for LuaSandbox which fails with the same error.

This was fixed as part of T233004.

Fixed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/LoginNotify/+/952114

Thanks!

It's been this way since 2020?

Note that the error was thrown from a Kubernetes pod. We should confirm that php.ini is correct in Kubernetes. On bare metal hosts, it is set to 5000000.

Most likely opcache corruption. Errors started only 1ms after the request timeout at 12:36:03.013, and then there was a flood of segfaults over the next few hundred milliseconds, finishing with "[WARNING] failed processes threshold (40 in 60 sec) is reached, initiating reload" at 12:36:03.800.

We used to use TMH to play audio in Score, but I got rid of it in cf8a4f2133cb6a0f9fa4266ed93acea4f0729667 and just used <audio> instead, because TMH was too tightly integrated with file uploads to reliably play a thing that wasn't an uploaded file, and its audio support was very poor.

Although, come to think of it, it's a somewhat implausible story, since you would think <img alt=""> would do the job well enough. It's possible the logo div was a hack for some other non-CSS client and I'm confusing it with Monobook's screen reader hacks.

In T188756#4074891, @Volker_E wrote:

@tstarling Where does “deliberately hidden” come from? https://www.powermapper.com/tests/screen-readers/labelling/a-title-img-no-alt/ tells a different story about link+title attribute combinations.

When I tested this locally using GlobalBlocking, I found that auto-creation actually succeeds. The IP address is globally blocked, but that is not actually checked for in CheckBlocksSecondaryAuthenticationProvider. There's a comment which says it is the responsibility of authorizeCreateAccount(), but that is not called.

I can't reproduce it anyway, because I don't know how you load a user script from meta in a way that would trigger this error. I tried checking Northern Moonlight's user scripts, but there was nothing helpful there. Most user common.js pages just use importScript() or importScriptURI(), neither of which use mw.loader.impl() as far as I can tell.

Although it looks like everything is on wmf.22 now, so too late. It should be fixed already.

Sorry, I thought we wouldn't need that cherry pick because the rest of the branch wasn't merged yet -- didn't look closely enough. I will deploy it now.

• Set up CentralAuth with two pilot wikis and one non-pilot wiki with reservedPattern as described above.
• Set up $wgForeignUploadTargets and use it to perform a foreign upload as a new normal user.

Ideally tests should be done with various combinations of the wikis being in the same and different cookie domains. But I don't really want to add more wikis to my CentralAuth farm if I can help it, since there's not much automation. It's easier to repeat the tests with different $wgAutoCreateTempUser configurations. I have four wikis, which are analogous to loginwiki, enwiki, frwiki and enwikiquote.

• On non-pilot wikis, the temp user prefix will be reserved with $wgAutoCreateTempUser['reservedPattern'], so it will not be possible to manually create a local account which has the same name as a global temp user. This seems prudent and was a design assumption for T307064 which introduced reservedPattern.
• Make UserNameUtils::isUsable() return false if isTempReserved() is true but isTemp() is false. So temp user reservations will be analogous to $wgReservedUsernames.
• Thus CentralAuthSessionProvider and CentralAuthTokenSessionProvider will reject the global session due to isUsable() returning false.
• In AuthManager::autoCreateUser(), where it says that we switched from isCreatable() to isValid() to support temp users, this will be changed again to instead call isUsable(). So auto-creation will be denied for foreign temp users on non-pilot wikis when it is requested by Special:CentralAutoLogin, ApiCreateLocalAccount, etc.

I'm working on a commit that avoids doing purges when the cache is empty.

This feature is called the hold-off period, and it is discussed in a few places in the source. There is likely some production impact. For 11 seconds after a purge, every request is a miss. OK, reasons are given for this. But also for 11 seconds following regeneration due to an empty cache, every request is a miss except in the first 50-100ms after each regeneration. This does not seem to be justified. It dates back to 2015 a0cce5e4b63d9607f4561298d346695bd4d1c31c. This detail was not discussed in Gerrit.