Great, I'm a little bit busy, professionally speaking but, I'll try to land it this week !

Nobody replied for months. I basically like the disclaimer and I basically trust your test in your production. Thanks.

I suggest to accept with "whatcouldgowrong" \o/

Thanks bob. Just to say that I care about your code proposals and sorry if this needed extra time.

(Indeed I agree we cannot bulk edit things - PHP 8.1 is a nightmare - see T15190)

Interestingly, Gerrit does similar things, and does not mention the problematic URI value in case it's somehow already clear:

Probably this was moved?

Cool. Two things:

(maybe this happens if you have just one provider and it's an OAuth)

Interestingly this cannot be reproduced in any public Phorge:

Other edges have probably been eliminated in the past. For example this one:

I sincerely thought that "Obsolete" was the good one semantically.

I think a dedicated policy for "Can send messages" would be better, to cover more cases. It would be strange that all participants must also be allowed to edit all settings.

Is this only related to the "Authored on ..." field, as far as you can see?

I cannot reproduce anymore. This does not happen anymore in latest master.

Sorry if I've found two additional interesting things:

From a product POV, I agree with @valerio.bozzolan - there is (sometimes) some information on commits that would be nice to index in a search engine - comments, mostly.

I don’t think revert I’d needed but the comment should probably be removed or updated. I’d like to understand why it was deemed hard to do but the solution here doesn’t seem that hard. Maybe it’s more difficult than it appears, or was robots.txt standard later updated in a way that makes this easier, or maybe Phab URLs changed in a way that made this easier but this was never updated, etc.

I'm not an important stakeholder, but I would like to share that in my installation https://gitpull.it I would like to have commits indexed as default as it happened as default and as it happens in GitHub and GitLab. So I'm now sincerely trying to understand how to restore the old behavior without keeping my own fork of Phorge if needed.

Thanks for the deep inspection! Tested and I agree

• Remove extra <tt> in bulk job dialog

In D25473#13673, @valerio.bozzolan wrote:

So this is the only blocking Test that is failing:

Visit /maniphest/, shift+click on at least 1 Task, click on Bulk Edit Selected, Continue, see the popup

If someone strongly feels that I should revert, please say so - thanks! :)

Valerio: Uhm, I'm sorry, I had not seen your comment here before I landed the patch (as I had checked my Differential page instead of my notifications).

Side note, we are introducing the possibility to share this kind of very creative and confusing URLs (that are safe from the point of view of XSS but) that could be an attracting point for lamers to generate confusing user messages inside Phorge itself, like:

Thanks for the additional debugging and finding steps to reproduce! In this case, I'd prefer not to log the issue and to only show a message to the user explaining what's the issue, including their requested bogus encoding.

Heh, very good catch! I can confirm, yes.

Thanks for landing!

This is something that may be not appreciated by some people.

Happy you fixed!

Thanks for the deeper investigation! I can confirm that with the last version and following my original steps to reproduce, no stuck daemon task about the mock looping due to an exception on http://phorge.localhost/daemon/ anymore, so I'm going to land this.

One of my favourite updates in my workplace was to store counts of feature usage per month in a table. Move forward 3 years, and whenever we need to update some code in a feature, we check for it in the table, and if it's not been used in 3 years we just scrap the feature.

Skip lint (javelinsymbols)

Great suggestions

• Implemented celerity_generate_unique_node_id()
• Implemented .collapsible-content > ul.remarkup-list CSS

Loved the comment, as otherwise I wouldn't have understood what was going on in the CSS. I've reworded it - hope that's OK.

JavaScript: ✅ I damn like your proposal and I think it is very OK, non-plus-ultra

I am quite happy with this now. Spent way too long on it, so I hope you like it too!

• Refactored the PHP code to build the collapsible button & div into buildCollapsibleIndex()
• Define the javelin call config first, then refer to it - saves defining the variables independently!
• Create an icon field in the button, allowing for open and close icons to be set
• Make both the icon and the text optional in the collapsible javascript - if you just want an icon, fine. Just text, also fine. Neither - sure!

So this is the only blocking Test that is failing:

We have just a minor issue but with a potentially working fix, and it's ready to land

I don't know. Let's see :)

The problem there is that by requiring collapsiblejx to have knowledge of the 'collapsible-toggler-label', we introduce tight coupling to it's caller. (see how adding 'jx' on to the end of collapsiblejx has made it uniquely identifiable?)
A better solution would be to have the collapsiblejx config contain 'btn_open_icon' and 'btn_close_icon' icons. I'll have a go...

avoid micro-optimizations that make this unreadable

• move duplicated button text into a variable
• improve formatting

• Renamed button/div IDs to remove spaces
• Renamed diffusion-collapsible-container to collapsiblejx
• Added margin-top: 10px; in the collapsible content - this overrides the higher up margin-top: 0px
• Moved webroot/rsrc/js/application/diffusion/collapsible-container.js to webroot/rsrc/js/core/behavior-collapsiblejx.js
• Renamed the JX from diffusion-collapsible-container to collapsiblejx - the single word collapsible was a bit too common if you 'git grep' for it.

As already noted we were affected by this margin-top: 0; that was making the TOC a bit too much attached to the button:

• sorry if I introduced an ID for just the label: now replace with a non-unique Javelin Sigil
• tried to fix the margin-top thing