@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Replace old hard-coded URI-based "changes saved" jank with new overgeneralized cookie-based "changes saved" jank
Summary:
Ref T13515. Settings currently has some highly specialized code for rendering "Changes saved." messages. The "saved" state is communicated across a redirect-after-POST by adding `/saved/` to the end of the URI.
This isn't great. It needs a lot of moving pieces, including special accommodations in routing rules. It's user-visible. It has the wrong behavior if you reload the page or navigate directly to the "saved" URI.
Try this scheme, which is also pretty sketchy but seems like an upgrade on the balance:
- Set a cookie on the redirect which identifies the form we just saved.
- On page startup: if this cookie exists, save the value and clear it.
- If the current page started with a cookie identifying the form on the page, treat the page as a "saved" page.
This supports passing a small amount of state across the redirect-after-POST flow, and when you reload the page it doesn't keep the message around. Applications don't need to coordinate it, either. Seems somewhat cleaner?
Test Plan: In Firefox, Safari, and Chrome: saved settings, saw a "Saved changes" banner without any URI junk. Reloaded page, saw banner vanish properly.
Maniphest Tasks: T13515
Differential Revision: https://secure.phabricator.com/D21144
+128
-28
+2
src/__phutil_library_map__.php
+2
src/__phutil_library_map__.php
···
3251
3251
'PhabricatorEditEngineMFAInterface' => 'applications/transactions/editengine/PhabricatorEditEngineMFAInterface.php',
3252
3252
'PhabricatorEditEngineNameTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineNameTransaction.php',
3253
3253
'PhabricatorEditEngineOrderTransaction' => 'applications/transactions/xaction/PhabricatorEditEngineOrderTransaction.php',
3254
+
'PhabricatorEditEnginePageState' => 'applications/transactions/editengine/PhabricatorEditEnginePageState.php',
3254
3255
'PhabricatorEditEnginePointsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEnginePointsCommentAction.php',
3255
3256
'PhabricatorEditEnginePreambleTransaction' => 'applications/transactions/xaction/PhabricatorEditEnginePreambleTransaction.php',
3256
3257
'PhabricatorEditEngineProfileMenuItem' => 'applications/search/menuitem/PhabricatorEditEngineProfileMenuItem.php',
···
9714
9715
'PhabricatorEditEngineMFAEngine' => 'Phobject',
9715
9716
'PhabricatorEditEngineNameTransaction' => 'PhabricatorEditEngineTransactionType',
9716
9717
'PhabricatorEditEngineOrderTransaction' => 'PhabricatorEditEngineTransactionType',
9718
+
'PhabricatorEditEnginePageState' => 'Phobject',
9717
9719
'PhabricatorEditEnginePointsCommentAction' => 'PhabricatorEditEngineCommentAction',
9718
9720
'PhabricatorEditEnginePreambleTransaction' => 'PhabricatorEditEngineTransactionType',
9719
9721
'PhabricatorEditEngineProfileMenuItem' => 'PhabricatorProfileMenuItem',
+15
src/aphront/AphrontRequest.php
+15
src/aphront/AphrontRequest.php
···
30
30
private $controller;
31
31
private $uriData = array();
32
32
private $cookiePrefix;
33
+
private $submitKey;
33
34
34
35
public function __construct($host, $path) {
35
36
$this->host = $host;
···
914
915
return $future;
915
916
}
916
917
918
+
public function updateEphemeralCookies() {
919
+
$submit_cookie = PhabricatorCookies::COOKIE_SUBMIT;
920
+
921
+
$submit_key = $this->getCookie($submit_cookie);
922
+
if (strlen($submit_key)) {
923
+
$this->clearCookie($submit_cookie);
924
+
$this->submitKey = $submit_key;
925
+
}
926
+
927
+
}
928
+
929
+
public function getSubmitKey() {
930
+
return $this->submitKey;
931
+
}
917
932
918
933
}
+2
src/aphront/configuration/AphrontApplicationConfiguration.php
+2
src/aphront/configuration/AphrontApplicationConfiguration.php
+1
-1
src/applications/auth/application/PhabricatorAuthApplication.php
+1
-1
src/applications/auth/application/PhabricatorAuthApplication.php
+7
src/applications/auth/constants/PhabricatorCookies.php
+7
src/applications/auth/constants/PhabricatorCookies.php
···
63
63
const COOKIE_INVITE = 'invite';
64
64
65
65
66
+
/**
67
+
* Stores a workflow completion across a redirect-after-POST following a
68
+
* form submission. This can be used to show "Changes Saved" messages.
69
+
*/
70
+
const COOKIE_SUBMIT = 'phfrm';
71
+
72
+
66
73
/* -( Client ID Cookie )--------------------------------------------------- */
67
74
68
75
+1
-1
src/applications/settings/application/PhabricatorSettingsApplication.php
+1
-1
src/applications/settings/application/PhabricatorSettingsApplication.php
+12
-4
src/applications/settings/editor/PhabricatorSettingsEditEngine.php
+12
-4
src/applications/settings/editor/PhabricatorSettingsEditEngine.php
···
128
128
return PhabricatorPolicies::POLICY_ADMIN;
129
129
}
130
130
131
-
public function getEffectiveObjectEditDoneURI($object) {
132
-
return parent::getEffectiveObjectViewURI($object).'saved/';
133
-
}
134
-
135
131
public function getEffectiveObjectEditCancelURI($object) {
136
132
if (!$object->getUser()) {
137
133
return '/settings/';
···
251
247
}
252
248
253
249
return parent::getValidationExceptionShortMessage($ex, $field);
250
+
}
251
+
252
+
protected function newEditFormHeadContent(
253
+
PhabricatorEditEnginePageState $state) {
254
+
255
+
if ($state->getIsSave()) {
256
+
return id(new PHUIInfoView())
257
+
->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
258
+
->appendChild(pht('Changes saved.'));
259
+
}
260
+
261
+
return null;
254
262
}
255
263
256
264
}
-1
src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php
-1
src/applications/settings/panel/PhabricatorEmailPreferencesSettingsPanel.php
-1
src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php
-1
src/applications/settings/panel/PhabricatorPasswordSettingsPanel.php
···
206
206
$algo_box = $this->newBox(pht('Password Algorithms'), $properties);
207
207
$form_box = id(new PHUIObjectBoxView())
208
208
->setHeaderText(pht('Change Password'))
209
-
->setFormSaved($request->getStr('saved'))
210
209
->setFormErrors($errors)
211
210
->setBackground(PHUIObjectBoxView::WHITE_CONFIG)
212
211
->setForm($form);
+41
-5
src/applications/transactions/editengine/PhabricatorEditEngine.php
+41
-5
src/applications/transactions/editengine/PhabricatorEditEngine.php
···
1036
1036
$fields = $this->buildEditFields($object);
1037
1037
$template = $object->getApplicationTransactionTemplate();
1038
1038
1039
+
$page_state = new PhabricatorEditEnginePageState();
1040
+
1039
1041
if ($this->getIsCreate()) {
1040
1042
$cancel_uri = $this->getObjectCreateCancelURI($object);
1041
1043
$submit_button = $this->getObjectCreateButtonText($object);
1044
+
1045
+
$page_state->setIsCreate(true);
1042
1046
} else {
1043
1047
$cancel_uri = $this->getEffectiveObjectEditCancelURI($object);
1044
1048
$submit_button = $this->getObjectEditButtonText($object);
···
1069
1073
1070
1074
$validation_exception = null;
1071
1075
if ($request->isFormOrHisecPost() && $request->getBool('editEngine')) {
1076
+
$page_state->setIsSubmit(true);
1077
+
1072
1078
$submit_fields = $fields;
1073
1079
1074
1080
foreach ($submit_fields as $key => $field) {
···
1154
1160
1155
1161
$field->setControlError($message);
1156
1162
}
1163
+
1164
+
$page_state->setIsError(true);
1157
1165
}
1158
1166
} else {
1159
1167
if ($this->getIsCreate()) {
···
1252
1260
$box_header->addActionLink($action_button);
1253
1261
}
1254
1262
1263
+
$request_submit_key = $request->getSubmitKey();
1264
+
$engine_submit_key = $this->getEditEngineSubmitKey();
1265
+
1266
+
if ($request_submit_key === $engine_submit_key) {
1267
+
$page_state->setIsSubmit(true);
1268
+
$page_state->setIsSave(true);
1269
+
}
1270
+
1271
+
$head = $this->newEditFormHeadContent($page_state);
1272
+
$tail = $this->newEditFormTailContent($page_state);
1273
+
1255
1274
$box = id(new PHUIObjectBoxView())
1256
1275
->setUser($viewer)
1257
1276
->setHeader($box_header)
···
1259
1278
->setBackground(PHUIObjectBoxView::WHITE_CONFIG)
1260
1279
->appendChild($form);
1261
1280
1262
-
// This is fairly questionable, but in use by Settings.
1263
-
if ($request->getURIData('formSaved')) {
1264
-
$box->setFormSaved(true);
1265
-
}
1266
-
1267
1281
$content = array(
1282
+
$head,
1268
1283
$box,
1269
1284
$previews,
1285
+
$tail,
1270
1286
);
1271
1287
1272
1288
$view = new PHUITwoColumnView();
···
1291
1307
return $page;
1292
1308
}
1293
1309
1310
+
protected function newEditFormHeadContent(
1311
+
PhabricatorEditEnginePageState $state) {
1312
+
return null;
1313
+
}
1314
+
1315
+
protected function newEditFormTailContent(
1316
+
PhabricatorEditEnginePageState $state) {
1317
+
return null;
1318
+
}
1319
+
1294
1320
protected function newEditResponse(
1295
1321
AphrontRequest $request,
1296
1322
$object,
1297
1323
array $xactions) {
1324
+
1325
+
$submit_cookie = PhabricatorCookies::COOKIE_SUBMIT;
1326
+
$submit_key = $this->getEditEngineSubmitKey();
1327
+
1328
+
$request->setTemporaryCookie($submit_cookie, $submit_key);
1329
+
1298
1330
return id(new AphrontRedirectResponse())
1299
1331
->setURI($this->getEffectiveObjectEditDoneURI($object));
1332
+
}
1333
+
1334
+
private function getEditEngineSubmitKey() {
1335
+
return 'edit-engine/'.$this->getEngineKey();
1300
1336
}
1301
1337
1302
1338
private function buildEditForm($object, array $fields) {
+47
src/applications/transactions/editengine/PhabricatorEditEnginePageState.php
+47
src/applications/transactions/editengine/PhabricatorEditEnginePageState.php
···
1
+
<?php
2
+
3
+
final class PhabricatorEditEnginePageState
4
+
extends Phobject {
5
+
6
+
private $isCreate;
7
+
private $isSubmit;
8
+
private $isError;
9
+
private $isSave;
10
+
11
+
public function setIsCreate($is_create) {
12
+
$this->isCreate = $is_create;
13
+
return $this;
14
+
}
15
+
16
+
public function getIsCreate() {
17
+
return $this->isCreate;
18
+
}
19
+
20
+
public function setIsSubmit($is_submit) {
21
+
$this->isSubmit = $is_submit;
22
+
return $this;
23
+
}
24
+
25
+
public function getIsSubmit() {
26
+
return $this->isSubmit;
27
+
}
28
+
29
+
public function setIsError($is_error) {
30
+
$this->isError = $is_error;
31
+
return $this;
32
+
}
33
+
34
+
public function getIsError() {
35
+
return $this->isError;
36
+
}
37
+
38
+
public function setIsSave($is_save) {
39
+
$this->isSave = $is_save;
40
+
return $this;
41
+
}
42
+
43
+
public function getIsSave() {
44
+
return $this->isSave;
45
+
}
46
+
47
+
}
-15
src/view/phui/PHUIObjectBoxView.php
-15
src/view/phui/PHUIObjectBoxView.php
···
7
7
private $background;
8
8
private $tabGroups = array();
9
9
private $formErrors = null;
10
-
private $formSaved = false;
11
10
private $infoView;
12
11
private $form;
13
12
private $validationException;
···
71
70
$this->formErrors = id(new PHUIInfoView())
72
71
->setTitle($title)
73
72
->setErrors($errors);
74
-
}
75
-
return $this;
76
-
}
77
-
78
-
public function setFormSaved($saved, $text = null) {
79
-
if (!$text) {
80
-
$text = pht('Changes saved.');
81
-
}
82
-
if ($saved) {
83
-
$save = id(new PHUIInfoView())
84
-
->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
85
-
->appendChild($text);
86
-
$this->formSaved = $save;
87
73
}
88
74
return $this;
89
75
}
···
324
310
$header,
325
311
$this->infoView,
326
312
$this->formErrors,
327
-
$this->formSaved,
328
313
$exception_errors,
329
314
$this->form,
330
315
$this->tabGroups,