Skip to content

Commit

Permalink
systemd/services: Fix event handler race leading to wrong page switch
Browse files Browse the repository at this point in the history
Event handlers are all async. It sometimes happened that clicking on a
unit in the list, or the test changing the URL to a particular unit, for
attempting to open the details page. This triggered the
`onOptionsChanged` handler, which  would reset the path back to the
list. With some added debug logging, the logs look like this:

> ServicesPageBody render list for path []
> Clicked on service in list, going to condtest.service
> cockpit.location.get; window.location.hash "#/?type=service"
> cockpit.Location.go: changed window.location.hash to #/condtest.service?type=service
-> wait: ph_collected_text_is("#statuses .status","Not runningAutomatically starts")
> cockpit.location.get; window.location.hash "#/condtest.service?type=service" creating new last_loc
> ServicesPageBody onOptionsChanged; cockpit.location {"url_root":"","path":["condtest.service"],"options":{"type":"service"},"href":"/condtest.service?type=service"} merged options {"type":"service"} calling go to services list; path prop []

^ i.e. here the onOptionsChanged fired, and reset the path back to `[]`.
We can't use `this.props.path` either, as that hasn't been updated yet.

> cockpit.location.get; window.location.hash "#/condtest.service?type=service" unchanged last_loc
> cockpit.Location.go: changed window.location.hash to #/?type=service
> locationchanged {"type":"service"}
> usePageLocation changed {"url_root":"","path":[],"options":{"type":"service"},"href":"/?type=service"}

So the details page often was not actually opened. Fix that by not
hardcoding the target path to `[]`, but using the current one instead.
This allows event handlers to be delayed even until after a page change.

Fixes cockpit-project#18443
  • Loading branch information
martinpitt authored and mvollmer committed Mar 6, 2023
1 parent 3ea7c47 commit 69d23d9
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions pkg/systemd/services/services.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class ServicesPageBody extends React.Component {
if (!currentOptions.name)
delete currentOptions.name;

cockpit.location.go([], currentOptions);
cockpit.location.go(cockpit.location.path, currentOptions);
}

componentDidMount() {
Expand Down Expand Up @@ -1024,7 +1024,7 @@ const ServicesPage = () => {

const activeTab = options.type || 'service';
const owner = options.owner || 'system';
const setOwner = (owner) => cockpit.location.go([], Object.assign(options, { owner }));
const setOwner = (owner) => cockpit.location.go(cockpit.location.path, Object.assign(options, { owner }));

if (owner !== 'system' && owner !== 'user') {
console.warn("not a valid location: " + path);
Expand All @@ -1041,7 +1041,7 @@ const ServicesPage = () => {
<ServiceTabs activeTab={activeTab}
tabErrors={tabErrors}
onChange={activeTab => {
cockpit.location.go([], Object.assign(options, { type: activeTab }));
cockpit.location.go(cockpit.location.path, Object.assign(options, { type: activeTab }));
}} />
<FlexItem align={{ default: 'alignRight' }}>
{loggedUser && loggedUser !== 'root' && <ToggleGroup>
Expand Down

0 comments on commit 69d23d9

Please sign in to comment.