Summary: Type in PHPDoc @param versus type in method signature mismatch. Thus correct PHPDoc.
Test Plan: Read the PHPDoc and the signature and compare types.
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/D25977
Summary:
Per PHPStan static code analysis, currently
`Instanceof between PhabricatorSearchService and PhabricatorMySQLSearchHost will always evaluate to false`.
Per discussion in the task and per testing, make the check work.
Closes T15891
Test Plan: Probably set up a Phorge instance with recent MariaDB/MySQL, and set up a Phorge instance with ancient DB versions (MySQL 5.6.3 or older; MariaDB 10.0 or older) not supporting FULLTEXT indexes for InnoDB tables?
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15891
Differential Revision: https://we.phorge.it/D25975
Summary: Fixes T16028
Test Plan:
Assert that `#js-draw-lib` (or any other project name with one or two characters followed by a non-word) links correctly. It was previously parsed as `#js -draw-lib`.
Repeat the test plan for D25838 and observe it still works
Reviewers: aklapper, O1 Blessed Committers, bekay, valerio.bozzolan
Reviewed By: O1 Blessed Committers, bekay, valerio.bozzolan
Subscribers: mainframe98, bekay, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16028
Differential Revision: https://we.phorge.it/D25959
Summary:
Per rP719dd6d3f44245e935b21291a59338a819938c49 and previous rP6cedd4a95cfc23a1679400fb64863c49f24f2306, remove the `search_documentfield` table from initial creation (`quickstart.sql`) and Diviner database docs.
Pointed out in Q82.
Additional context:
MySQL 5.6.4 released 2011-12-20 "now supports FULLTEXT indexes for InnoDB tables" per https://forums.mysql.com/read.php?3,506409 and per https://downloads.mysql.com/docs/mysql-5.6-relnotes-en.pdf pages 230 and 233.
Per https://www.oracle.com/us/support/library/lifetime-support-technology-069183.pdf page 29, "Extended Support Ends" for "MySQL Database 5.6" in "Feb 2021".
MariaDB went for version 10 instead of 5.6 per https://mariadb.org/mariadb-10-0-and-mysql-5-6/; MariaDB 10.0 saw its End of Life date on "31 Mar 2019" per https://mariadb.org/about/#maintenance-policy
Test Plan:
* As an admin, go to `/config/issue/` which triggers `PhabricatorSetupCheck::runNormalChecks()` and thus runs database checks.
* Install a fresh instance of Phorge which runs quickstart.sql, for example setting `"storage.default-namespace": "test"` inside `conf/local/local.json` and see that things still works, like Phriction search by name etc.
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25895
Summary:
`-moz-background-clip: padding` syntax was only supported until Firefox 3.6 according to https://web.archive.org/web/20130827100732/https://developer.mozilla.org/en-US/docs/Web/CSS/background-clip. Firefox 4.0 released 03/2011 supports standard `background-clip: padding-box`.
The same docs also imply that `-webkit-background-clip: padding-box` currently used by Phorge was never supported by browsers. Per https://caniuse.com/mdn-css_properties_background-clip_padding-box, Chrome 4 released 10/2010 supports standard `background-clip: padding-box`.
Compare current `webroot/rsrc/css/phui/phui-image-mask.css` which does not have this old vendor-prefixed syntax.
Test Plan:
* Open the Conpherence top bar dropdown, select "Persistent Chat", see that its `.jx-scrollbar-handle` based scrollbar still works as expected.
* Read CSS docs. Probably install ancient browser versions if you don't trust documentation. Try https://jigsaw.w3.org/css-validator/ and don't spot issues.
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/D25970
Summary:
* `.phui-timeline-preview-header` has never been used since introduction in rPdb66cd830d1b.
* Only use of `.phui-timeline-change-details` was removed in direct followup rP4e8e035de006.
Test Plan: Grep 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/D25918
Summary:
APC saw its last release in 2012.
It has been discontinued as PHP 5.5+ includes OpCache.
It has been superseded by APCu for object caching.
See https://pecl.php.net/package-info.php?package=APC
Phorge requires PHP 7.2.
Thus remove any references to APC.
Also note that "If APCu is installed, it reports that APC is installed" does not hold true anymore: In PHP 8.3, "php -m" correctly reports "apcu".
Test Plan:
* Make sure APCU is not installed, see `PHP Extension "APCu" Not Installed" under "Unresolved Setup Issues" as an admin
* Apply patch, still see same issue listed.
* Install `php-pecl-apcu` package on Fedora system; see that now the issue is not listed anymore under "Unresolved Setup Issues"
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/D25950
Summary:
`PhabricatorEmailPreferencesSettingsPanel::getAllEditorsWithTags()` queries for all classes extending `PhabricatorApplicationTransactionEditor` and having `getMailTagsMap()` to then pull their editor application class. Thus return the correct editor application class.
Followup to rP821708414e.
`grep -r "('PhabricatorApplicationTransactionEditor'" .` shows only a single result in the Phorge codebase (this `PhutilClassMapQuery->setAncestorClass('PhabricatorApplicationTransactionEditor')` call) so I assume there will be no unwanted side effects.
Closes T16002
Test Plan:
* Uninstall Audit on `/applications/view/PhabricatorAuditApplication/`
* Check for `Audits` section on `/settings/panel/emailpreferences/`
* Re-install Audit and try random actions on a commit like: Accept commit, Raise a concern, Change Project Tags, award token, Subscribe, Unsubscribe, Resign as Auditor, no regressions
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16002
Differential Revision: https://we.phorge.it/D25894
Summary:
Configuring SMTP for Gmail as described will trigger
"535-5.7.8 Username and Password not accepted. Learn more at 535 5.7.8 https://support.google.com/mail/?p=BadCredentials"
GMail only supports OAuth and 2FA/App Passwords these days; the "less secure apps" option (which would be needed for the described SMTP setup) has been removed.
Test Plan:
Follow the docs; set up `cluster.mailers` config setting in Phorge; run `./bin/mail send-test --to admin --subject subject < ~/foo` and fail.
Optionally, watch https://www.youtube.com/watch?v=Y_u5KIeXiVI for your personal corporate excitement.
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/D25911
Summary:
Passing `null` to `preg_match()` is deprecated behavior since PHP 8.1.
Some clients do not pass a `User-Agent` HTTP header so `$agent` is null.
Thus only call `preg_match()` when `$agent` is set.
```
ERROR 8192: preg_match(): preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated
#0 preg_match(string, NULL) called at [<phorge>/src/view/page/PhabricatorStandardPageView.php:629]
```
Closes T15829
Test Plan: Access a task page etc in a browser which does not set a `User-Agent` HTTP Header string.
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: mainframe98, slip, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15829
Differential Revision: https://we.phorge.it/D25868
Summary:
The W3C CSS validator throws `Value Error : cursor normal is not a cursor value : normal`.
`normal` is not a valid value for `cursor`; likely `default` was meant.
See https://developer.mozilla.org/en-US/docs/Web/CSS/cursor
Test Plan: Paste `phui-oi-list-view.css` into https://jigsaw.w3.org/css-validator/ and read some CSS docs.
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/D25919
Summary:
This is PHPDoc for a class header. Thus there is no `@param` to pass.
Phorge specific `@task info` was meant here. See other examples in classes.
Test Plan: Read and grep 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/D25954
Summary:
Avoid "has invalid return type obj" errors when running static code analysis (PHPStan).
See https://docs.phpdoc.org/guide/references/phpdoc/types.html
Test Plan: Run PHPStan before and after applying this change (and its corresponding Arcanist change), get 15 errors less.
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/D25952
Summary:
This reverts commit fd6118bfa6.
Closes T16040
Reopens T15513
Test Plan:
Write text in a Conpherence room and press Enter to send the text.
Test again, as non-participant, on a public chat. Still works.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16040, T15513
Differential Revision: https://we.phorge.it/D25969
Summary:
If the content of a hovercard contains another hovercard trigger, the initial hovercard will be closed when entering the inside trigger with the mouse. You can see the bug with every differential hovercard which contains reviewers. But there are other cases too: For example if project descriptions use mentions or other project tags.
This fix is pretty easy and returns inside the mousemove handler if the trigger node has a closest ancestor with the class "jx-hovercard-container".
Closes T16029
Test Plan:
See if a hovercard is closed if you enter following inside the hovercard:
- reviewers inside differential hovercard
- people mentions or other project tags inside project hovercard
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16029
Differential Revision: https://we.phorge.it/D25965
Summary:
DiffusionCommitQuery passed duplicate CommitterIdentity values to PhabricatorRepositoryIdentityQuery, making logs slightly more noisy.
Thus run `array_unique()` before passing.
Closes T16032
Test Plan: Go to http://phorge.localhost/diffusion/commit/query/all/ and look at SQL query in Dark Console before and after applying patch.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16032
Differential Revision: https://we.phorge.it/D25964
Summary:
Also show the time of a file uploaded, not only the date.
This is consistent for example with exposing the creation time of Maniphest tasks.
Closes T16030
Test Plan: Go to http://phorge.localhost/file/ while logged in and logged out, see that `Uploaded by testadmin on Wed, Apr 9` changed to `Uploaded by user on Wed, Apr 9, 10:48 AM`.
Reviewers: O1 Blessed Committers, mainframe98, valerio.bozzolan
Reviewed By: O1 Blessed Committers, mainframe98, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16030
Differential Revision: https://we.phorge.it/D25961
Summary:
See T13065 and rP1e2bc7775bc4.
Intended to cover the gap left by D25899.
The actual amount of impacted installations should
be 0, but the migration makes sure all pastes have
a mail key regardless.
Test Plan:
* Create a paste
* Check `SELECT * FROM phabricator_paste.paste;` and `SELECT * FROM phabricator_metamta.metamta_mailproperties;` on the database
* Run `./bin/storage upgrade`
* Check that the mailKeys have been moved to `mailProperties` in the `phabricator_metamta.metamta_mailproperties` table and the `mailKey` column is gone from the `phabricator_paste.paste` table
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25900
Summary:
Use Unicode abbreviation `μs` for microseconds instead of ASCII `us` which made me wonder for a while what that is.
`mb_chr` is available since PHP 7.2.0 (which Phorge requires). Entering μ directly makes lint bark about "Bad Charset".
Test Plan:
Go to http://phorge.localhost/herald/transcript/1/profile/ and look at values in the "Cost" column.
Works with PHP 8.3; no clue which prehistoric PHP versions may have problems.
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/D25942
Summary:
See rPaaab1011e5a464ea94c182001a7fe867b19bae5a.
`AphrontView::setUser()` is deprecated.
Save a few CPU cycles and nuclear power plants.
Test Plan:
Read the code.
Even better, intentionally introduce an error in `AphrontView::setUser()`
and browse random Phorge pages and then replace the call accordingly (as
there are numerous `setUser()` methods across the codebase and we don't
want no explosions).
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/D25880
Summary:
Switch the two column layout of AphrontSideNavFilterView (used in pretty much most views) from table to flex display. The former CSS rendering could lead to overflowing of the entire page on small screens if very wide unbreaking content is displayed (example described here: T15809).
While I was at it I have found some unused code. The CSS file `phabricator-nav-view.css` consisted almost only of `.phabricator-side-menu-home` class selectors. This is an ancient class (there at least since 2013) not used anymore:
https://we.phorge.it/source/phorge/browse/master/src/applications/directory/controller/PhabricatorDirectoryController.php;919bd4a03499305093d8882952a9dd3dac1c55a9$169
So I have removed this CSS component and saved the few still used declarations to the main `phui-basic-nav-view.css`.
There was some unused code in `AphrontSideNavFilterView.php` too.
Fixes T15809
Test Plan:
Look at this video to understand the problem:
F2178277
Go in a Task comment and mention a task with a very-long title. Have phd enabled to generate feeds. Visit homepage with feeds:
See that you cannot reproduce anymore.
Browse pages with the right nav bar with many devices and look if the layout looks fine and not broken.
Tested the homepage, scaling from mobile to desktop, using:
- Firefox 124 64bit on GNU/Linux (snap)
- Chromium 124 64bit on GNU/Linux (snap)
- ...
Tested other elements, scaling from mobile to desktop, still working correctly:
- popup dialogs
- right floating menu
- top curtain
- notifications
- Conpherence chats
- unresolved setup issues
- search omnibox
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15809
Differential Revision: https://we.phorge.it/D25619
Summary:
This extension was deprecated in PHP 5.5.0 and removed in PHP 7.0.0.
Given the requirement of PHP 7.2, it makes no sense to support this
extension.
Closes T16024
Test Plan: Grep for both `mysql` and `mysqli` and look for outdated references.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16024
Differential Revision: https://we.phorge.it/D25946
Summary: For explicitly mentioned Phorge class types in the function signature, use the same type in the PHPDoc for the parameter.
Test Plan: Compare classname type in function signature and in PHPDoc; run static code analysis.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25937
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/auth/xaction/PhabricatorAuthFactorProviderNameTransaction.php:20]
```
Closes T15992
Test Plan: After applying the changes, go to http://phorge.localhost/feed/transactions/ and check the entries related to setting up an MFA provider.
Reviewers: O1 Blessed Committers, slip, avivey
Reviewed By: O1 Blessed Committers, slip, avivey
Subscribers: slip, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15992
Differential Revision: https://we.phorge.it/D25867
Summary:
Followup to 1d34238dc9.
Crossing fingers that nobody complains about a one-time logout after their six year long user session.
Closes T16025
Test Plan:
* Log in, log out, do stuff.
* More specifically, with my session expired, successfully logging in created a database row added to `phabricator_user.phabricator_session` as expected, and logout removed the row.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16025
Differential Revision: https://we.phorge.it/D25949
Summary: These private functions are not called within their class.
Test Plan:
Grep; run static code analysis.
Additionally, this still works:
arc unit src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php
These pages still work:
/conpherence/search/
/differential/
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/D25933
Summary:
PHP casts an array key which is a string that contains a decimal integer automatically into int type.
Quoting https://www.php.net/manual/en/language.types.array.php#language.types.array.syntax: "Strings containing valid decimal ints, unless the number is preceded by a + sign, will be cast to the int type."
As we check git branch names via `phutil_nonempty_string($key)`, this throws an exception.
Thus explicitly cast the int to string.
Closes T15640.
Test Plan:
* Have a git repository with a branch whose name consists of digits only and does not start with 0.
* Go to a commit which belongs both to the main/master branch and such a numeric branch via http://phorge.localhost/diffusion/ABCD/branches/main/;1234567890abcdef1234567890abcdef12345678
* Additionally, visit the commit and see that the "Branches" does not load as it also belongs to a numeric branch.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15640
Differential Revision: https://we.phorge.it/D25945
Summary:
`phid` is not a valid type. See https://docs.phpdoc.org/guide/references/phpdoc/types.html.
Thus set the param type to `string` and get a few less errors reported by PHPStan.
Test Plan: Read docs; run static code analysis.
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/D25940
Summary:
Replaces Authy and Google Authenticator, both proprietary with FreeOTP, Bitwarden Authenticator, Aegis, and 2FAS, all free and open-source.
Closes T16018
Test Plan:
- Setup TOTP factor in the Auth app.
- Setup the TOTP factor on your account.
- On both, **double check** the text for any typo or missing app. If any, click on {nav Request Changes}.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, Cigaryno, tobiaswiese, valerio.bozzolan, Matthew
Tags: #auth, #user-cigaryno
Maniphest Tasks: T16018
Differential Revision: https://we.phorge.it/D25934
Summary:
The W3C CSS validator throws `Value Error : overflow normal is not a overflow value : normal`.
`normal` is not a valid value for `overflow`, as browsers currently cannot parse it they fall back to the default `visible`, thus change it to `visible`.
See https://developer.mozilla.org/en-US/docs/Web/CSS/overflow
Test Plan:
Paste `phui-oi-list-view.css` into https://jigsaw.w3.org/css-validator/ and read some CSS docs.
Also, visit a page showing a PHUIObjectItemView, like the /phame/post/ page, and
have a object with a veeeeeery long name, and see absolutely no difference A/B,
both on desktop, medium and small screens.
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/D25921
Summary:
Per https://caniuse.com/ and MDN,
* `-ms-transform` superseded by unprefixed `transform` supported since IE 10 released 09/2012
* `@-ms-keyframes` superseded by unprefixed `@keyframes` supported since IE 10 released 09/2012
* `-ms-transition` superseded by unprefixed `transition` supported since IE 10 released 09/2012
Test Plan: None. Probably installing old browser versions if you don't trust documentation.
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/D25928
