Summary: This clears up a common source of type check errors when using php intelephense.
Test Plan: See the squiggles in vscode go away :)
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26039
Summary:
Adding class templates to `PhabricatorPolicyAwareQuery` and
`PhabricatorCursorPagedPolicyAwareQuery` to make `execute()` have the right
return signature, when the query classes are also annotated. Otherwise, the
return type is inferred as `PhabricatorPolicyInterface` as before.
Test Plan:
Add some annotations to query classes and see that `$query->execute()` infers
the more specific return type.
Diviner still works after ./bin/diviner generate. For example this page still exist:
/book/dev/class/PhabricatorCursorPagedPolicyAwareQuery/
And the method newResultObject() is now documented with the return type R|null.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26037
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/search/controller/PhabricatorSearchController.php:34]
```
Test Plan: Visit `http://phorge.localhost/search/?search:primary=true`
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26061
Summary:
These tags already use colors to differentiate. Use a tooltip for "NOT" query token tags like all the other types also do, because consistency.
Closes T16093
Test Plan:
1. Go to https://we.phorge.it/search/query/advanced/
2. In the `Query` field, enter `bleh =zsd +a NOT(x) "phrase like this" a -phasa ~amp title:fb "use meo" érr`.
3. Click `Search` button.
4. Look at the tags shown for the string `Searched For: ...`.
5. See that only red tags also use a (minus) icon.
6. Don't see a minus icon for the red token tag anymore, now get a tooltip for the red token tag.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16093
Differential Revision: https://we.phorge.it/D26057
Summary: I'm still trying to understand search a bit better. As a side effect I add PhpDocs.
Test Plan: Inspect parameter and return formats, types, content while searching in Phorge.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26042
Summary:
Warn users who already have MFA set up that adding a second Multi-Factor Auth will require entering both instead of being able to choose from one of them.
This is currently not clear. I was surprised by this, now I have another user also surprised.
Closes T16081
Test Plan:
1. As an admin, set up TOTP as an auth provider at http://phorge.localhost/auth/mfa/
2. As a user, add a first TOTP auth factor at http://phorge.localhost/settings/panel/multifactor/
3. As a user, try to add a second TOTP auth factor and see an additional sentence in the dialog
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16081
Differential Revision: https://we.phorge.it/D26028
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.
```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated
#0 PhabricatorOAuthServerTokenController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
```
See Q182
Test Plan: Read the code. `strlen` is used to get the length of an existing string and not to check for emptiness of a string. There is no string length comparison in the existing code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26034
Summary:
Avoid an AphrontParameterQueryException when running a typeahead project name query with `mustHaveColumns` when the search string is not the `prefix` of the project name.
Since rP9a1d59ad5bd51ef4bf56533db260cc9e11a54fe6, Phorge runs queries in two phases: First `prefix`, then `content`, to sort project results accordingly.
`PhabricatorTypeaheadDatasource::loadResultsForPhase()` first goes for the `prefix` phase. This query may return zero results.
Current code does not correctly handle this situation.
Thus make the code smoothly skip to the `content` query phase and not run a `PhabricatorProjectColumnQuery` with an empty project PHID array parameter to end up in an exception.
Closes T16068
Test Plan:
* Have a parent project named `Foo` with a milestone named `Sprint Chocolate Cake`
* Create a project workboard for that milestone named `Sprint Chocolate Cake`
* In a project with no workboard yet, select "Import Columns" to end up on http://phorge.localhost/project/board/1/import/
* See that typing the second string token (`Chocolate`) shows no autocomplete project proposals
* Click the Magnifier button to `Browse Projects`
* Start typing `Chocolate` and get an AphrontParameterQueryException
* Apply the patch
* See that typing the second string token (`Chocolate`) now shows autocomplete project proposals
* Click the Magnifier button to `Browse Projects`
* Start typing `Chocolate` and get project search results
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16068
Differential Revision: https://we.phorge.it/D26030
Summary: I'm trying to understand search a bit better. As a side effect I add PhpDocs.
Test Plan: Inspect parameter and return formats, types, content while searching in Phorge.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26033
Summary:
Remove non-standard macOS specific font smoothing CSS which should be unneeded in the age of Retina displays and be superseded anyway by system and browser settings.
Closes T16059
Test Plan: Run browsers under macOS. Which I do not have.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16059
Differential Revision: https://we.phorge.it/D25989
Summary:
Do not throw an exception when passing bogus parameter string values to hovercards. Instead, just convert to an empty array.
In consequence, see a proper hovercard correctly displaying `Unknown Object (????)` instead of an exception:
```
EXCEPTION: (TypeError) idx(): Argument #1 ($array) must be of type array, string given, called in /var/www/html/phorge/phorge/src/applications/search/controller/PhabricatorSearchHovercardController.php on line 37 at [<arcanist>/src/utils/utils.php:37]
```
See also similar rPa5384ca60470e56a5312d5a7147ddd3ffa2e75d8.
Closes T16075
Test Plan:
* Go to http://phorge.localhost/search/hovercard/?__path__=%2fsearch%2fhovercard%2f&cards={"layout":"foo"}
* Get an error message before applying the patch. Get an empty hovercard after applying the patch.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16075
Differential Revision: https://we.phorge.it/D26023
Summary:
Remove some of those small PNG files. Most of the flag icons are still used by the Flags application.
The `key_question.png` icon is covered by D26005.
Test Plan: Grep for the filenames.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26006
Summary: Closes T16082
Test Plan:
Visit this page:
http://phorge.localhost/calendar/event/edit/form/default/?start=1747749600&end=1747753200
No warning anymore from your Dark Console.
The Start and End fields are still prefixed with '2025-05-20', so, no regressions.
Reviewers: O1 Blessed Committers, mainframe98, aklapper
Reviewed By: O1 Blessed Committers, mainframe98, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16082
Differential Revision: https://we.phorge.it/D26029
Summary:
Set the `device-phone` CSS class also for Firefox on Android to (hopefully) avoid or decrease large layout shifts.
See https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/User-Agent/Firefox
Closes T16071
Test Plan:
Check if the given regex matches User-Agent strings.
Also, emulate a desktop and a mobile visit with cURL, and check their HTML body CSS classes:
lang=bash
curl --silent --user-agent "Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0" http://phorge.localhost | grep -Po '<body class=".*?"'
curl --silent --user-agent "Mozilla/5.0 (Android 11; Mobile; rv:138.0) Gecko/138.0 Firefox/138.0" http://phorge.localhost | grep -Po '<body class=".*?"'
Output before the change, observe that the second line is recognized as desktop by mistake:
lang=html,counterexample
<body class="device-desktop platform-linux phui-theme-blindigo phabricator-home"
<body class="device-desktop phui-theme-blindigo phabricator-home"
Output after the change, observe that the second line is finally recognized as mobile:
lang=html
<body class="device-desktop platform-linux phui-theme-blindigo phabricator-home"
<body class="device-phone device phui-theme-blindigo phabricator-home"
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16071
Differential Revision: https://we.phorge.it/D26021
Summary:
All implementations of `canLoadNamedObject($name)` in classes which extend `PhabricatorPHIDType` run a `preg_match()` on the `$name` parameter being passed.
Thus the parameter must always be a string.
(Spoiler: This isn't always the case currently.)
Test Plan: Read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26025
Summary:
Phorge does not expose the location of the last call in unhandled exceptions on the web page. One has to open the error log or console to find the location.
The error message in the yellow box comes directly from PHP's `$throwable->getMessage()`, and the stacktrace comes directly from PHP's `$throwable->getTrace()`.
Thus manually concatenate the error message string with `$throwable->getFile()` and `$throwable->getLine()` to display the location of the last call but strip the absolute path not to expose server details.
Closes T15689
Test Plan:
Introduce a random PHP error and load the Phorge page.
For example, add `$chart_view += $chart_panel;` in src/applications/project/controller/PhabricatorProjectReportsController.php and go to http://phorge.localhost/project/reports/1/
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15689
Differential Revision: https://we.phorge.it/D25955
Summary:
`revision-age-fresh`, `revision-age-old`, `revision-age-stale` were added in rPb50cdc6e439c4f2915a47cb40cd94425832c22a2.
Their usage was removed in rPd9f01d6fb7f50e707805bc059aa15dcafca05160 from `src/applications/differential/view/DifferentialRevisionListView.php`.
Test Plan: Grep the code for `revision-age`. No results.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25996
Summary:
Finally we have some auto-generated documentation for the page /conduit/method/transaction.search/ about the supported values of objectType.
This is useful since we have 70+ of them, and they are totally obscure to newcomers (and us, core team, lol).
{F3942084}
The already-existing exception messages now do not suggest anymore values that exist but are not supported (e.g. XACT).
Moreover, we handle an unmanaged exception in transaction.search, when the existing `objectType` is not supported (e.g. XACT).
Closes T16054
Closes T16057
Test Plan:
Visit the page /conduit/method/transaction.search/ and enjoy the new amazing auto-generated table.
From the same page, visit the test form and set objectType = "XACT" and Call Method. You do not see anymore this confusing exception:
get_class() expects parameter 1 to be object, null given
Instead, you finally see this informative exception message (similar to the already-existing ones):
In call to "transaction.search", specified "objectType" ("XACT") is not supported because it does not implement "newObject()". Valid object types are: ABND, ADEV, AINT, ANAM, ANET, ASRV, DIFF, DREV, BOOK, DRYB, FBAK, FITV, HMBD, HMCP, HMCS, HMBB, HRUL, HWBH, HWBR, TASK, NUAI, NUAQ, NUAS, CDTL, FPRV, AUTH, CTNM, AMSG, APAS, AKEY, BDGE, CEVT, CEXP, CIMP, CONF, CONP, CDWN, DSHB, DSHP, PRTL, FORM, FILE, LEGD, MCRO, APPE, OASC, OPKG, PPAK, PPUB, PVER, PSTE, USER, BLOG, POST, PHRL, PANL, PCOL, PROJ, WTRG, CMIT, RIDT, REPO, RURI, POLL, SPCE, PSET, BULK, PVAR, MOCK, AEML, ACNT, CART, PMRC, PAYM, PHPR, PSUB, WIKI, ANSW, QUES.
Additionally, you can verify that the new documentation and the new methods in general are NOT called for normal valid API requests, so, not affecting production performance. An example of a valid request is: objectType = "TASK".
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16054, T16057
Differential Revision: https://we.phorge.it/D25988
Summary: Trivial but as I was looking into this code it cannot hurt I guess...
Test Plan: Manually check classes/types of parameters.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26019
Summary:
The `E_STRICT` PHP constant is deprecated since PHP 8.4 per https://www.php.net/manual/en/migration84.deprecated.php.
Per https://wiki.php.net/rfc/deprecations_php_8_4#remove_e_strict_error_level_and_deprecate_e_strict_constant, the `E_STRICT` constant was still in use in-between PHP 7.0 and 7.4 for PHP's `mysqli` extension and PHP's `htmlentities()` function.
The `E_STRICT` notice was removed from PHP's `mysqli` extension in e895e96286 for PHP 7.4.0.
Since rP23a49eb403c9ea6c58f4ae2f22416e90a9d24c14, Phorge requires 7.2.25.
Phorge does not call PHP's `htmlentities()` function.
`E_STRICT` is a part of `E_ALL` since PHP 5.4.0 per https://github.com/php/php-src/blob/php-5.4.0/NEWS#L69, thus there is no gain in keeping it.
Closes T15989
Test Plan: None.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, keithzg, mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15989
Differential Revision: https://we.phorge.it/D25887
Summary: Original URI is a 404. Thus link to functional URIs.
Test Plan: Open a web browser, fill in URIs, see where you end up on your journey.
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26004
Summary:
* `config-options-current-value`
** Added in https://we.phorge.it/rP25ca17da46348d974ffa9bc451a0a81ed6babbcf, usage removed in https://we.phorge.it/rP530145ba3be62b6c54b8f3bc8aca09f2f369697d by deleting `src/applications/config/controller/PhabricatorConfigGroupController.php`
Test Plan: Grep the code for each CSS selector (`grep --exclude="*.css" -R 'some-search-string' .`), also do partial searches, get no results.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26002
Summary:
* slowvote-header-content
** Added in https://we.phorge.it/rP89ee928a51c72d9fda17a59f73b060a92cc0c665, usage removed in https://we.phorge.it/rPaa22a578d01b8b5c69c89b9dc4a370552534f6dd from `src/applications/slowvote/view/SlowvoteEmbedView.php`
Test Plan: Grep the code for each CSS selector (`grep --exclude="*.css" -R 'some-search-string' .`), also do partial searches, get no results.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26003
Summary:
* `maniphest-task-updated`
** Added in rPdf0c3df3cc6dbaa151e1b32ff4e01052eb3dad3e, usage removed in rP018de5dec742b6a38d02710670c1dd30101f5b9f by deleting `src/applications/maniphest/view/ManiphestTaskSummaryView.php`
Test Plan: Grep the code for each CSS selector (`grep --exclude="*.css" -R 'some-search-string' .`), also do partial searches, get no results.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26001
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/conpherence/query/ConpherenceFulltextQuery.php:58]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conpherence/query/ConpherenceFulltextQuery.php:73]
```
Closes T16062
Test Plan: Go to a Conpherence room and search for an existing string.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16062
Differential Revision: https://we.phorge.it/D25994
Summary:
`jxui-active`, `jxui-blue-background`, `jxui-example-container` and `jxui-red-border` were added in rP998d43e828b010ca0051ffab74108111826027f1.
Their usage was removed in rP7725d7cc45c7792610746b4a773e4cb12743d873.
Test Plan: Grep the codebase for `jxui`.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25993
Summary:
`ManiphestReportController` (which renders global `/maniphest/report/burn/`) wastes CPU cycles. Thus remove useless code.
It constructs table rows never to be shown: The code calculates rows and adds them via `$table = new AphrontTableView($rows)`, then defines `$panel = new PHUIObjectBoxView()` and does `$panel->setTable($table)` but a bit later `$panel = id(new PhabricatorProjectBurndownChartEngine())` overwrites that table and all its data.
This re-definition of `$panel` was introduced in rP5c1b91ab457db9f3db10d8cc5e07831512645ebb.
Its code creates a `PhabricatorProjectBurndownChartEngine` anyway, similar to `PhabricatorProjectReportsController`. So none of those expensive database queries are needed at all. Variables like `list($burn_x, $burn_y) = $this->buildSeries($data);` are never read since rPf8ebc71b8f217ed156f416ddb4cd028dcaa28174.
Bug: T16005
Test Plan:
1. Make sure `/applications/view/PhabricatorFactApplication/` is installed
2. Go to `/maniphest/report/burn/` for global view, go to some URL like `/maniphest/report/burn/?project=PHID-PROJ-1234567890abcdef` for per-project view
3. See that the "Burnup Rate" chart is rendered in the same way before and after applying this patch
4. Optionally, run `./bin/cache purge --all` (but seems not relevant here) and reload
5. Optionally, go to `/project/reports/1/` and as expected the burndown chart is the same as in step 2
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16005
Differential Revision: https://we.phorge.it/D25902
Summary:
Add "clip-path" as progressive enhancement for deprecated "clip" property to be more future-proof.
See the Phorge task for details.
Closes T16046
Test Plan: Probably browse Phorge pages which use FontAwesome icons, plus test with a screenreader software? (I didn't.)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16046
Differential Revision: https://we.phorge.it/D25983
