Skip to content

Remember every WMS URL msui loaded capabilities from - #3318

Open
joernu76 wants to merge 1 commit into
developfrom
i3317
Open

joernu76 wants to merge 1 commit into
developfrom
i3317

Conversation

@joernu76

Copy link
Copy Markdown
Member

A URL typed into the WMS URL box and confirmed with Enter was never stored: Enter inserts it into the combo box, so the success handler found it there and skipped saving. Only one URL was stored anyway.

msui now keeps every URL it loaded capabilities from in a list shared by all views, most recent first, and still reads the single URL older versions stored. Del or Backspace on a highlighted URL in the opened list forgets it; default servers stay, and a loaded server stays listed until it is unloaded.

Also fix the success handler leaving duplicate entries when a known URL is entered again without trailing slash, and add_wms_urls adding duplicates within its list. The tests reset the stored URLs before every test.

Refs #3317

Purpose of PR?:

Fixes #3317

A URL typed into the WMS URL box and confirmed with Enter was never
stored: Enter inserts it into the combo box, so the success handler
found it there and skipped saving. Only one URL was stored anyway.

msui now keeps every URL it loaded capabilities from in a list shared
by all views, most recent first, and still reads the single URL older
versions stored. Del or Backspace on a highlighted URL in the opened
list forgets it; default servers stay, and a loaded server stays listed
until it is unloaded.

Also fix the success handler leaving duplicate entries when a known URL
is entered again without trailing slash, and add_wms_urls adding
duplicates within its list. The tests reset the stored URLs before
every test.

Refs #3317

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@joernu76
joernu76 requested a review from ReimarBauer November 10, 2026 22:08
@joernu76

Copy link
Copy Markdown
Member Author

We might need to discuss this. The behaviour of MSS was not ideal when we configured it for the campaign yesterday.

@ReimarBauer ReimarBauer left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jörn

some hints by claude code-review, commented by me

#3318 fixes only part of #3317. The reported bug is fixed: a newly entered WMS URL now stays in the list. The issue's second request isn't done, which is to offer, on the first connection, to add the server to the default list (default_WMS in msui_settings.json). The PR still says "Fixes #3317", so merging it would close the issue with that part undone. joernu76 already wrote on the PR "We might need to discuss this". Either the second part goes into a follow-up issue, or the PR should say "Refs #3317" instead.

What the PR gets right:

  • The cause is identified correctly: pressing Enter put the typed URL into the list, so the success handler found it there and never saved it.
  • Every successful server connection now saves its URL, whatever the list shows.
  • The single URL that older versions stored is still read.

Bugs worth raising:

  1. Already open views don't see new or deleted URLs (wms_control.py:574). Each view reads the list once, when it opens. Load a new server in the top view, and an open side view won't list it until reopened. Delete a URL in one view, and it stays in the other. The docs, though, say the list is shared by all views.
  2. Default URLs can disappear (wms_control.py:1035). When the server answers with a slightly different address, e.g. a trailing /, the code removes or renames the entry that was used, even when it's a configured default. A renamed default can then be deleted with Del.
  3. Deleting with Del can seem to do nothing (wms_control.py:1063). If the deleted entry was the selected one, its URL stays in the input field. The next click on Get Capabilities loads it and saves it again.
  4. Secrets in URLs are kept indefinitely (wms_control.py:80). URLs are saved in full, with user, password or token, in plain text in msui-core.conf. Before, only the last URL was kept and overwritten each time. Now they stay until the user deletes each one with Del, which only a tooltip mentions.
  5. The list has no size limit and fills with URLs the user never typed (wms_control.py:80), such as autoplot servers and redirect targets.
  6. All views share one list (wms_control.py:573). Map servers loaded in the top view also show up in the side and linear views, where they offer no layers.

Smaller:
7. A URL list stored as a plain string falls apart into single characters (wms_control.py:71): list() is applied without a type check.
8. Saving the list deletes the old key recent_wms_url (wms_control.py:81). An 11.x installation using the same config file then loses its remembered URL.
9. select_wms_url looks up the same URL up to three times (wms_control.py:1028). This tangled logic is where bug 2 is.

For 1. we maybe need to update the lists on open views
For 2. and 3. and 9. we need a test and then a fix
For 4. We should exclude Urls with API Keys, passwords, token.
For 5. we should add a limit e.g. 20? and cycle then.
For 6. we maybe should have a check of valid data for the view. We should not show servers which don't provide data.

@ReimarBauer ReimarBauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New WMS URL retention

2 participants