Summary:
When creating a Differential diff manually via `/differential/diff/create/` in the web browser instead of using Arcanist, `setIsCreateTransaction(true)` for the transaction type `DifferentialDiffTransaction::TYPE_DIFF_CREATE` to avoid an empty `Transaction` field in the Feed and avoid `strncmp()` complaining about a null value being passed.
For gory details, see the comments in T15659.
```
ERROR 8192: strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/policy/query/PhabricatorPolicyQuery.php:314]
```
Closes T15659
Test Plan:
1. Create a valid diff via `/differential/diff/create/`
2. Go to `/feed/transactions/query/advanced/` and set `Object Types` to `Differential Diff` and click `Search`
3. See two entries in the `Transaction` column for the just created diff: One says "created this diff", the other one says "created this object with visibility" and is not an empty line anymore which triggered an exception.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15659
Differential Revision: https://we.phorge.it/D25517
Summary: See T15726
Test Plan: Go to `/applications/` and see that Fund and Phortune say "Deprecated".
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25533
Summary:
PhabricatorSearchTextField.php's parent class PhabricatorSearchField.php defines an (empty) `protected function validateControlValue($value)`.
Override this function in PhabricatorSearchTextField.php by disallowing arrays (as the only data type which cannot be casted into a text value).
Closes T15714
Test Plan: Apply D25518; then manually edit the code in `ManiphestTaskSearchEngine.php` by defining an array and passing that array to the Page Size text field via `id(new PhabricatorSearchTextField())->setDefaultValue($empty_array)`, then go to `/maniphest/query/advanced/` and see the error message.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15714
Differential Revision: https://we.phorge.it/D25519
Summary:
When `$value` is `null`, do not pass `$value` down the stack in `buildControl()` to ultimately end up with `json_decode` complaining. Instead, just skip this call.
```
EXCEPTION: (RuntimeException) json_decode(): Passing null to parameter #1 ($json) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=customOAuthUrlencodeNull, ref.master=788098096e11, ref.customOAuthUrlencodeNull=4f0f2043b7e9), phorge(head=customFieldDate, ref.master=bcfcd9acfc12, ref.customFieldDate=ae8cbe84252d)
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#1 <#2> json_decode(NULL, boolean) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php:44]
#2 <#2> PhabricatorStandardCustomFieldPHIDs::setValueFromStorage(NULL) called at [<phorge>/src/infrastructure/customfield/field/PhabricatorCustomField.php:895]
#3 <#2> PhabricatorCustomField::setValueFromApplicationTransactions(NULL) called at [<phorge>/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php:70]
#4 <#2> PhabricatorCustomFieldEditField::buildControl() called at [<phorge>/src/applications/transactions/editfield/PhabricatorEditField.php:385]
```
Closes T15602
Test Plan: After applying these three changes and creating a custom field with `"type": "users"` under `/config/edit/maniphest.custom-field-definitions/`, the website `/transactions/editengine/maniphest.task/view/5/` renders correctly in the browser, showing "This is a preview of the current form configuration."
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15602
Differential Revision: https://we.phorge.it/D25390
Summary:
`::class` is available since PHP 5.5 (5.5 is a minimal requirement by Phorge): https://www.php.net/manual/en/language.oop5.basic.php#language.oop5.basic.class.class
It makes finding code using IDEs easier; see discussion in D25500.
Thus replace all string return values with returning the `::class` constant instead, with one exception: 'PhabricatorSettingApplication' in `PhabricatorUserPreferencesSearchEngine.php` does not exist and makes arc lint fail so this string remained unchanged.
Also note that two occurrences were wrapped in `pht()` for reasons I do not know.
List of functions whose return value get updated in this code change:
* getApplicationClassName()
* getAdapterApplicationClass()
* getDatasourceApplicationClass()
* getEditorApplicationClass()
* getEngineApplicationClass()
* getPHIDTypeApplicationClass()
* getQueryApplicationClass()
cf. T15158
Test Plan: Too broad - click around, basically.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15158
Differential Revision: https://we.phorge.it/D25524
Summary:
Revert rPf8ffa393c4427f8b9026075e6dc490990a12df12 (D25021). This Commit caused more grief then fun.
Ref T15630
Test Plan: Site should still be readable on most platforms
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15630
Differential Revision: https://we.phorge.it/D25530
Summary:
Add `getQueryApplicationClass()` to all `*TransactionQuery.php` classes similar to other `*Query.php` classes having the same function, and make the parent function in `PhabricatorApplicationTransactionQuery.php` abstract.
In the future, this will enable excluding transaction query results based on their underlying application (for example if an application has been uninstalled) to mitigate the problem of overheated search results. See https://we.phorge.it/T15642 for context.
The only callers of `getQueryApplicationClass()` are in `src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php` and `src/applications/policy/__tests__/PhabricatorPolicyTestCase.php`.
See T15642
Test Plan:
Patch changes only one existing code place, thus check if related pages still work as expected:
* Go to http://phorge.localhost/feed/
* Go to http://phorge.localhost/feed/transactions/
* On http://phorge.localhost/feed/transactions/ , click `Edit Query` and set `Object Types` to `Application` etc.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15642
Differential Revision: https://we.phorge.it/D25500
Summary:
Ref T15645
The very helpful "Locate File" input in Diffusion was so far only visible in the homepage route of any repository (`/repository`).
With this revision you can now locate a file from every browsed directory and in any selected commit.
The finder was already "directory sensitive" meaning: if you are trying to locate a file from within a browsed directory, only the children of this path will be searched.
For the searching in a specified commit (for example: https://we.phorge.it/source/phorge/browse/master/src/;05f4d5071fdca02123bd1ff4c0935b847c7f9963), I had to do a little JS magic adding the commit to the URI on the client side.
Test Plan: Checkout, browse through your repos with Diffusion trying to find files. (I tested only with Git repos.)
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15645
Differential Revision: https://we.phorge.it/D25521
Summary:
The text in remarkup has a fixed line height. The headers will have the same fixed line height, even if their font size can be bigger than this line height. Especially on mobile this results in ugly sticked together lines which are less readable.
This revision will give the header a flexible line height of `1.3`. Unitless means relative to the font size.
| Before | After |
|-----------|-----------|
| {F717418} | {F717419} |
(I have moved the same margin attribute for every `h{2-6}` into the new declaration.)
Close T15716.
Test Plan: Just checkout and look in wiki articles with headers to see if everything looks nice and neat.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15716
Differential Revision: https://we.phorge.it/D25523
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/phortune/provider/PhortunePaymentProvider.php:99]
```
Closes T15725
Test Plan: See steps in T15725
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15725
Differential Revision: https://we.phorge.it/D25528
Summary:
When trying to set a custom project profile image while the PHP GD extension is not installed, use the same logic which already exists in `PhabricatorFilesComposeAvatarBuiltinFile.php` to set the default project image. This stills display an unhelpful error message `This server only supports these image formats: .` but avoids an exception trying to call GD's `imagecreatefromstring()`.
```
EXCEPTION: (Error) Call to undefined function imagecreatefromstring() at [<phorge>/src/applications/files/builtin/PhabricatorFilesComposeIconBuiltinFile.php:131]
```
Closes T15326
Test Plan:
1. Remove the php-gd (and potentially gd) packages on your system; restart httpd
2. Go to http://phorge.localhost/project/manage/1/
3. Select `Edit Picture` in the sidebar on the right to go to the `Edit Project Picture` at http://phorge.localhost/project/picture/1/
4. Set a custom icon and color and click the `Save Image` button
5. Get `This server only supports these image formats: .` but no exception anymore
Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan
Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15326
Differential Revision: https://we.phorge.it/D25515
Summary:
This revision increases the usability of the typeahead results of the main search (on the right in the top bar).
The result texts wont't overflow in a reasonable way.
| Before | After |
|-----------|-----------|
| {F715296} | {F715385} |
The result title (red box) will now break so the user can read and understand it.
The result type (blue box) will be cut off inside the padding with an ellipsis.
See T15715
Test Plan:
Check it out and use the main search in the top bar to create typeahead result with very long titles ;)
Nothing changes for short titles.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15715
Differential Revision: https://we.phorge.it/D25520
Summary:
{T15473} created a very small but annoying regression. Every embedded images has now an margin at the bottom:
{F718091}
I don't think this is intentional. If you click on the margin the image itself is opened not the lightbox. This revision removes the margin.
Test Plan: Look at tasks with embedded images and see if there is still an margin.
Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper
Reviewed By: O1 Blessed Committers, valerio.bozzolan, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25522
Summary:
When `files.viewable-mime-types` is not set, `getViewableMimeType()` passes `null` to `preg_match()` which is deprecated behavior since PHP 8.1.
Only call `preg_match()` when there are some MIME types to compare.
```
ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/storage/PhabricatorFile.php:974]
```
Closes T15710
Test Plan: Go to a user profile and try to upload a custom profile picture in BMP format.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15710
Differential Revision: https://we.phorge.it/D25516
Summary:
Add a documentation link in a specific Config page, that is:
Config > Setting > user.custom-field-definitions
Test Plan:
Visit the page and click on the new link:
- /config/edit/user.custom-field-definitions/
Visit this page that was the inspirational page:
- /config/edit/maniphest.custom-field-definitions/
No nuclear implosions.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25507
Summary:
Add CSS to ellipsize long labels and disable text wrapping
Closes T15707
Test Plan:
Set a long custom "action" label in `/config/edit/maniphest.points/`; go to a Maniphest task; open all items in the "Add Action..." dropdown.
Example config:
```
{
"enabled": true,
"label": "Points",
"action": "Set Points Yeah Points Points Much Points"
}
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15707
Differential Revision: https://we.phorge.it/D25511
Summary:
At the moment the "SSH Key Details" page lists only two minimal information:
- Key Type (Example: 'ssh-rsa')
- Created (Example: 'Wed, Dec 13, 5:13 PM')
After this change it shows also the original Public Key. It becomes something like:
- Public Key (Example: 'ssh-rsa AAAB3....... mariana@localhost')
- Created (Example: 'Wed, Dec 13, 5:13 PM')
This is just a minimal UX change.
Note that you can already see the Public Key by clicking "Edit".
So, nothing new is shown.
Context:
Premising that Phorge encourages to Revoke your decommissioned / deprecated
/ compromised SSH keys with a very lovely "Revoke" Workflow...
Root problem:
If you Revoke one of your keys, and if you then want to double-check that
SSH Public Key again (if you want to see its original body), you then discover
that you cannot see it anymore. In fact, the only way is the Edit button, but
Editing is disabled for any Revoked Key. In this kind of situation, it's not
nice to have to involve your favorite database administrator, to inspect the
'phabricator_auth.auth_sshkey' database table, to just be able to see your SSH
Public Key again.
After this change, you can easily self-audit your Revoked SSH Public Keys,
and your beloved DBA has extra free time. Win-win situation.
Closes T15692
Test Plan:
- visit your SSH Keys, open one, and check the new Details with your big eyes
- do the dame while juggling two ping pong balls (is anybody reading this?)
- do the same on a Revoked key
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15692
Differential Revision: https://we.phorge.it/D25495
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php:14]
```
Closes T15691
Test Plan: After configuring a custom `select` field, try to create a task via a Maniphest form using that custom field.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15691
Differential Revision: https://we.phorge.it/D25494
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:484]
```
Closes T15687
Test Plan:
Unknown. Definitely requires having custom fields defined, then playing with creating tasks using forms which expose these fields and going to `/maniphest/query/all/`.
See also D25487.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15687
Differential Revision: https://we.phorge.it/D25492
Summary: `x` is not a valid unit.
Test Plan: Open a Maniphest task in the web browser; inspect the CSS of the label of any items in the "Add Action" dropdown.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25510
Summary:
U2F/FIDO are open standards for inexpensive two-factor authentication hardware that are widely used for website authentication.
These keys are available since OpenSSH 8.2 (2020-02-14).
https://www.openssh.com/txt/release-8.2https://security.stackexchange.com/q/240991/260234
Ref Q96
Test Plan: Check the added keys. They exactly match the output of the command 'ssh -Q key'.
Reviewers: O1 Blessed Committers, antonia, avivey
Reviewed By: O1 Blessed Committers, antonia, avivey
Subscribers: avivey, tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25508
Summary: Fixes T15698
Test Plan:
1. Uninstall Packages via `/applications/view/PhabricatorPackagesApplication/`
2. Go to `/diffusion/commit/`, expand the Query form, and see that `Packages` is not shown anymore
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15698
Differential Revision: https://we.phorge.it/D25506
Summary:
Do not assume that all items under `/resources/builtin/projects/` are image files (some can be subdirectories). Only add PNG files to the map.
Could be nicer to use `is_file()` or `is_dir()` but we are dealing only with string representations of files and folders.
Closes T15696
Test Plan: Go to `/project/picture/1/` and click the `Choose Icon and Color...` button; check last icon shown under "Choose Icon".
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15696
Differential Revision: https://we.phorge.it/D25499
Summary:
Do not trigger an "Undefined index" error in PhabricatorCustomFieldStorage when calling the `project.search` Conduit API without passing search parameters when the local project_customfieldstorage table has additional fields configured not requested in the API call.
Closes T15688
Test Plan: See above; `curl -k -s -X GET http://phorge.localhost/api/project.search?api.token=api-xxxxxxxxxxxx | jq -r` for comparison
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15688
Differential Revision: https://we.phorge.it/D25493
Summary:
The docs don't specify how to make the second sshd daemon start on every reboot.
I've added a unit that I use in my setup to the docs.
Ref T15467
Test Plan: Check that the docs look alright.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15467
Differential Revision: https://we.phorge.it/D25497
Summary: This typo appeared in the docs and I thought I'd fix it
Test Plan: See that the typo at src/applications/config/option/PhabricatorCoreConfigOptions.php is gone
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25496
Summary:
When given `$encoding` is invalid, catch the exception to show a proper error message and make the server logs provide more hints.
```
EXCEPTION: (RuntimeException) mb_convert_encoding(): Illegal character encoding specified at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#1 <#2> mb_convert_encoding(string, string, string) called at [<phabricator>/src/applications/files/document/PhabricatorTextDocumentEngine.php:73]
```
Closes T15624
Test Plan: Open a URL which passes a bogus encoding value as parameter, like `/source/somerepository/browse/master/README.md?as=source&encode=TROLOLOL`
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15624
Differential Revision: https://we.phorge.it/D25418
Summary:
Before this change, the right navigation panel (Curtain) had info without Hovercard support.
After this change, all fields, like Subscribers, Tags, Referenced Files etc. have Hovercards.
If you meet an UX problem with a specific field with Hovercard, you can disable that specific
Hovercard, calling PHUICurtainObjectRefView::setHovercarded(false).
This is an example, now with Hovercard support:
{F342785}
Closes T15577
Test Plan:
No nuclear implosions when, on desktop and with a mouse:
Browse Maniphest Task and mouse hover Authored By, Assigned To, Subscribers, Referenced Files, ...
Browse Phriction document and mouse Tags, Referenced Files, Subscribers, ...
Browse single commit and mouse hover Referenced Files, Subscribers, ...
Browse Ponder question and mouse hover Tags, Referenced Files, Subscribers, ...
Browse Differential revision and mouse hover Tags, Referenced Files, Subscribers, ...
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15577
Differential Revision: https://we.phorge.it/D25424
Summary:
Capture Deprecation Warnings, collect them into cache, and show them as a Setup Issue for admins to see and report back to us.
This only captures a sample of the traces, so not to overwhelm users (and RAM. and us) with reports.
Requires D25388. Refs T15554.
Test Plan: Run some flows that are known to bring up Deprecation Warnings. See them as a Setup Issue! Click little triangles to see details.
Reviewers: O1 Blessed Committers, Matthew
Reviewed By: O1 Blessed Committers, Matthew
Subscribers: revi, Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15554
Differential Revision: https://we.phorge.it/D25440
Summary:
This is a follow-up to D25425, where these improvements to the CLI prompt markers were discussed.
Changes included in this revision:
- Build all prompts the same way
- Remove space after the prompt marker (add it via CSS instead)
- Add server path prefix
- Make the prompt unselectable
Test Plan:
- Visit any of the setup issue pages, e.g. <PHORGE_URL>/config/issue/auth.config-unlocked/ (after ensuring that the corresponding issue is present — in this case, by doing `./bin/auth unlock`)
- For example, Deactivate all PHP extensions to trigger each /config/issue/extension.gd/ etc.
- For example, update at least up to `dc10a7e69ea3` to see the database upgrade tip etc.
- Confirm that the command line prompts now include the path prefix
- Confirm that selecting the command via double-click (or click-and-drag) does not select the prompt
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #ux, #config
Differential Revision: https://we.phorge.it/D25466
Summary:
Metadata of Phame blog posts includes the 'updated' date of a blog post. Make them also include the original 'published' date.
Patch written by @20after4 from:
https://phabricator.wikimedia.org/rPHAB3de500bfc845759d6da82180df0adfc12f973463
Closes T15686
Test Plan:
* Go to the Atom feed of blog 1 at `/phame/blog/feed/1/` and look at this `<feed>`'s source
* Apply patch and do the same, see additional `<published>` entry just like the `<updated>`
* Edit the last blog post in blog 1, then check Atom feed and verify that the published and updated date stamp differ as expected
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, 20after4, Cigaryno
Maniphest Tasks: T15686
Differential Revision: https://we.phorge.it/D25490
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php:33]
```
Closes T15684
Test Plan: After configuring a custom `text` field and a dashboard panel to query and listed the latest created tasks, access the panel.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15684
Differential Revision: https://we.phorge.it/D25488
Summary:
Use user's preferred font for inline code snippets,
like we already do for block code snippets.
Test Plan:
Set a non-default font in Settings → Display Preferences → Monospaced Font.
Make sure that it applies to both single-tick (inline) and triple-tick (block)
code snippets in comments.
For example, enter:
42px Tahoma
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15682
Differential Revision: https://we.phorge.it/D25486
Summary:
**Fix Conpherence messages overlapping header in mobile mode**
Before:
{F413544}
After:
{F413546}
**Make Conpherence input box be one line even in desktop mode**
Before:
{F413541}
After:
{F413540}
This second change, in particular, is motivated by the fact that pressing `Enter` sends the message, whereas the multi-line box gives the impression that the `Enter` key would simply introduce a line break in the message. (That's still possible via `Shift`+`Enter`, btw.)
Test Plan:
- Visit a Conpherence room with some content in mobile mode;
scroll down and notice that the messages no longer slightly overlap the header.
- Visit a Conpherence room in desktop mode;
verify that the input box now has the height of a single line.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #conpherence
Differential Revision: https://we.phorge.it/D25485
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
In the other case, do not call `json_decode()` when passing null to it.
```
EXCEPTION: (RuntimeException) json_decode(): Passing null to parameter #1 ($json) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#1 <#2> json_decode(NULL, boolean) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php:44]
```
```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php:76]
```
Closes T15683
Test Plan: After configuring a custom `select` field, access a task.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15683
Differential Revision: https://we.phorge.it/D25487
Summary:
Replace call to undefined `renderHandleLink()` with `renderHandle()`.
Additionally, pass `head_key($new)` instead of `key($new)`. This might not be needed strictly speaking for this very issue but should not harm either as several images in a mock can be renamed at once.
```
EXCEPTION: (Error) Call to undefined method PholioImageSequenceTransaction::renderHandleLink() at [<phorge>/src/applications/pholio/xaction/PholioImageSequenceTransaction.php:32]
```
Closes T15680
Test Plan:
* Rename the title of an image in a Pholio mock.
* Go to `/feed/transactions/query/all/` which now renders instead of showing an error, and shows two entries:
* `user renamed an image (newname.jpg) from oldname.jpg to newname.jpg.` and `user updated an image's (newname.jpg) sequence.`
(However, separate T15679 will still show up in the error log after this change.)
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15680
Differential Revision: https://we.phorge.it/D25482
Summary:
While testing https://secure.phabricator.com/D21864 I ran into some issues getting mercurial HTTP access working. Using wireshark I confirmed that my local mercurial 6.4 was not including command arguments as HTTP headers but in the querystring.
I didn't dig too deep into understanding when/why this started happening. The protocol documents this in [[ https://repo.mercurial-scm.org/hg/file/tip/mercurial/helptext/internals/wireprotocol.txt | wireprotocol.txt ]].
>Command arguments can be sent multiple ways. The simplest is part of the URL query string using ``x-www-form-urlencoded`` encoding (see Python's ``urllib.urlencode()``. However, many servers impose length limitations on the URL. So this mechanism is typically only used if the server doesn't support other mechanisms.
Based on that either the mercurial on the server is really old (it's 6.1.1 tho) or maybe some other parsing/info passing in Phab's handling of the wire protocol is causing the client to downgrade the wire protocol support.
Cherry-picked from:
https://secure.phabricator.com/D21867https://secure.phabricator.com/rP0b6e758978a9691bd5ad25db4aa4c4301640a9a9
Test Plan: Host mercurial repo using HTTP, test push/pull.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25471
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/xaction/PhabricatorFileAltTextTransaction.php:59]
```
Closes T15678
Test Plan: Add and remove alt text on an image file; then go to `/feed/query/all/`
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15678
Differential Revision: https://we.phorge.it/D25481
Summary:
This commit removes ChatLog entirely. All of the application files are removed, and the migrations used are stubbed out. I stubbed the migrations as that allows for existing installs to make no changes, but new installs will not create the database.
Fixes T15126
Test Plan: Loaded up http://phorge.local/chatlog and confirmed the 404. Loaded up http://phorge.local/applications/view/PhabricatorChatLogApplication and confirmed the 404. Created a new database prefix and ran `bin/storage upgrade` against it, confirmed that the chatlog database was not created. Restored another prefix (an old one) and ran `bin/storage upgrade` and confirmed database was not deleted.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Cigaryno
Maniphest Tasks: T15126
Differential Revision: https://we.phorge.it/D25480
Summary:
With PHP 8.1+ it is not possible to import a commit if the commiter field is not properly defined
Indeed, if the committer is not properly defined, strlen(null) is called, causing a deprecation warning, elevated to exception.
Using strlen() to check string validity is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as a replacement.
Fix T15629
Test Plan:
- Push a commit to an observed subversion repository
- Import it via the phorge/bin/repository reparse COMMIT_ID --importing
- The commit should be properly imported and available in Diffusion
Reviewers: O1 Blessed Committers, Sten, valerio.bozzolan
Reviewed By: O1 Blessed Committers, Sten, valerio.bozzolan
Subscribers: Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15629
Differential Revision: https://we.phorge.it/D25423
Summary:
With PHP 8.1+ it is not possible to view a commit if the author field is not properly defined
Indeed, if the commit author is not properly defined, strlen(null) is called, causing a deprecation warning, elevated to exception.
Using strlen() to check string validity is deprecated since PHP 8.1. Phorge adopts phutil_nonempty_string() as a replacement.
Fix T15628
Test Plan:
- Push a new commit on a subversion repository (since T15629 is not yet addressed)
- Visualize the commit
- You should not get a RuntimeException
Reviewers: O1 Blessed Committers, Sten, valerio.bozzolan
Reviewed By: O1 Blessed Committers, Sten, valerio.bozzolan
Subscribers: Sten, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15628
Differential Revision: https://we.phorge.it/D25422
Summary:
This call prevents users to view a commit in subversion repositories
Indeed, if commiter and/or author field is not properly defined strlen is call with a NULL argument.
Using strlen to check string validity is deprecated since PHP 8.1, phorge adopts phutil_nonempty_string() as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
Fix T15610
Test Plan:
- Sign in (if needed)
- Open a diffusion SVN repository
- Open a commit without user name and or email
- You should be able to view the commit
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15610
Differential Revision: https://we.phorge.it/D25400
Summary:
Update the Application Detail view and List View to show a unified set of Badges (Deprecated, etc.), show PHIDs and Monograms on the Application Detail view, allow Applications to register Monograms.
Example of the page /applications/view/PhabricatorDiffusionApplication/:
{F393393}
T15568
Test Plan:
1. Visit /applications/ and see Deprecated badges etc.
2. Visit various Configure buttons from that list and see Monograms, Badges, PHIDs etc.
3. Enjoy screenshots in the comments of this Diff
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey, speck
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey, speck
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Cigaryno
Differential Revision: https://we.phorge.it/D25362
Summary:
Just a small set of replacements in locations that seem innocuous (user-facing messages, documentation, etc.)
Ref T15006
Test Plan:
Nothing should change in terms of behavior. The places where these changes were made should now say "phorge".
Example tests:
- Manage a single User and click on Delete User and see the popup
- Run a test email and check the output
./bin/mail send-test --to username
- Visit /maniphest/, shift+click on at least 1 Task, click on Bulk Edit Selected,
Continue, see the popup
- See the mentioned documentation with your big eyes. Eyes do not explode \o/
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15006
Differential Revision: https://we.phorge.it/D25473
Summary:
Phorge already sets `Disallow: /diffusion/` and `Disallow: /source/`.
Thus consequently also disallow accessing specific commits via `/r*`.
See https://secure.phabricator.com/T4610 for previous discussions.
Closes T15670
Test Plan:
Go to `/robots.txt` in the web browser.
Cross fingers that more webcrawlers abide by RFC 9309.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15670
Differential Revision: https://we.phorge.it/D25474
Summary:
Paste provides line anchor links in every single line of a paste.
If webcrawlers follow these links, they index the very same Paste again.
Thus disallow in robots.txt to reduce unneeded traffic and indexing time.
Closes T15662
Test Plan:
Go to `/robots.txt` in the web browser.
Cross fingers that more webcrawlers abide by RFC 9309.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15662
Differential Revision: https://we.phorge.it/D25461
Summary:
Premising that the `$old` and `$new` variables are 1-element arrays defined as PHID=>title,
this can cause `renderValue()` repeatedly fail when passing an array instead of its value.
Thus pass `head($old)` instead, to get the first value - that is the only one, even if you rename
multiple images (since this Transaction is about a single Mockup image).
Closes T15646
Test Plan:
* Have `phd` running
* Create a Pholio mockup with at least one image
Edit the Pholio mockup and:
1. rename the Titles of an image
2. rename a single Image
3. rename no image
No nuclear implosions. You still see a lovely Feed mentioning each rename.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15646
Differential Revision: https://we.phorge.it/D25441
Summary:
If the author and the committer are the same person, do not show them twice.
From:
UsernameFoo committed <commit hash>: <commit msg> (authored by UsernameFoo).
To:
UsernameFoo committed <commit hash>: <commit msg>
This only affects the feed.
| Before | After |
|-----------|-----------|
| {F342758} | {F342764} |
Closes T15528
Test Plan: Do some mixed commits and visit /feed/query/all/. No nuclear implosions.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15528
Differential Revision: https://we.phorge.it/D25421
