avoid to introduce a PHP 8 issue (thanks @mainframe98)

I've done my best to try to understand what we are talking about, so, I've created a nonsense X grid made in this way to catch changes:

Be bold Andre, feel free to land 🚀

(Damn Phorge that auto-claims also for wontfix)

Re "implement in php" - AFAICT, there's no built-in support for websockets in php, but I found at least 2 pure-php implementations out there that might work.

Maybe we can wontfix this. The current "workaround" D25485 by @waldyrious is just great and effectively fixed the super-confusing root problem.

Thanks Andre who fixed all the sub-tasks and deserves all the glory here. I cannot imagine more things to be covered, so, boldly closing.

Thanks. We can update the "We didn't yet release a fix on Phorge" and the "We'll release" since D25967 it's now fixed in master 👍

In J6#66, @Cigaryno wrote:

Thanks for this! Is this any related to the security vulnerability warning I got with npm install last month, which prompted me to run npm audit fix?

Thanks for this! Is this any related to the security vulnerability warning I got with npm install last month, which prompted me to run npm audit fix?

Testing welcome.
Input from CSS folks welcome.

From a quick look it seems that clip: rect(0 0 0 0) is very much the same as clip: rect(1px, 1px, 1px, 1px) (make things invisible and only found by screenreaders), and clip-path: inset(50%) (covering each side by 50%) is very much the same outcome as clip-path: inset(100%).

In D25967#26245, @avivey wrote:

@aklapper want to land this?

Possible ways to reduce risk for future issues:

• add a Setup Check that runs npm audit
• remove node, use php-based websocket implementation

@aklapper want to land this?
I figure users need to just run npm audit fix to be safe, and then fix the mess it did on the git diff.

Ah...I may take another look (jmeador: Feel of course also very free to commandeer this back to you)

Ah sorry for maybe stepping on toes, and welcome back! :)

Seems @jmeador is AWOL thus updating per last comment

seems @jmeador is AWOL thus boldly commandeering

Digging a bit further, rP600a3e3b7c2de5d93644e0410cd354ea6752949d implies that methods and files should not be indexed at all and not show up in the results. See also T16045#21931.

A quick fix would be removing the single line $query->withIsDocumentable(true); in https://we.phorge.it/source/phorge/browse/master/src/applications/diviner/controller/DivinerFindController.php;85f51c54303fe50ebc09ee0b652033a8a9f29ab1$45
as that'll allow results for Methods.

also manually bump version in package.json; then running npm install results in another bump in package-lock.json

Both are good questions. I only put here the diff which npm created. First question: Very likely Yes.
(Note that I have no knowledge in this area and don't even know why both package-lock.json and package.json are needed.)

1. Can we specify the .10 in packages.json itself?
2. Do installs need upgrade instructions to complete the upgrade?

As a general rule, I prefer the have the abstractions as much as possible, to allow extensions to do things.
In this case, an abstraction would also make this feature easier to enable/disable, which I think is desired.

Chris has asked me to pick this up as he'd like to see this implemented.

If the CSS rule should not change anything, why does it change something?

Yuuum. As I mentioned downstream ( https://phabricator.wikimedia.org/T380361#10778444 ) I think probably, fortunately, nobody noticed this in a real world, since it's complicated to cause this condition using the API, and probably impossible from the web interface, but this still seems somehow a reasonable workflow and not dead code and the change makes sense. Thaaaanks

Remove "return;" lines